George Dunlap
2012-Aug-14 17:33 UTC
[TESTDAY] xl cpupool-create segfaults if given invalid configuration
# xl cpupool-create ''name="pool2" sched="credit2"'' command line:2: config parsing error near `sched'': syntax error, unexpected IDENT, expecting NEWLINE or '';'' Failed to parse config file: Invalid argument *** glibc detected *** xl: free(): invalid pointer: 0x0000000001a79a10 *** Segmentation fault (core dumped) Looking at the code in xl_cmdimpl.c:main_cpupoolcreate() it calls: * xlu_cfg_init() * xlu_cfg_readdata() if the readdata() fails, it calls xlu_cfg_destroy() before returning. Other callers to readdata() seem to call exit(1) if readdata() fails. So is readdata() leavnig the config in an inconsistent state? Or is config already cleaned up? -George
Ian Campbell
2012-Aug-15 16:35 UTC
Re: [TESTDAY] xl cpupool-create segfaults if given invalid configuration
On Tue, 2012-08-14 at 18:33 +0100, George Dunlap wrote:> # xl cpupool-create ''name="pool2" sched="credit2"'' > command line:2: config parsing error near `sched'': syntax error, > unexpected IDENT, expecting NEWLINE or '';'' > Failed to parse config file: Invalid argument > *** glibc detected *** xl: free(): invalid pointer: 0x0000000001a79a10 *** > Segmentation fault (core dumped) > > Looking at the code in xl_cmdimpl.c:main_cpupoolcreate() it calls: > * xlu_cfg_init() > * xlu_cfg_readdata() > > if the readdata() fails, it calls xlu_cfg_destroy() before returning.I think the issue is the parser has: %destructor { xlu__cfg_set_free($$); } value valuelist values which frees the current "setting" but does not remove it from the list of settings. xlu_cfg_destroy then walks the list of settings and frees it again. This stuff is more of an Ian J thing but I wonder if when we hit the syntax error that $$ still refers to the last value parsed, which we think we are finished with but actually aren''t? i.e. we''ve already stashed it in the cfg and the reference in $$ is now "stale". IOW, I wonder if the patch at the end is the right thing to do. It seems to work for me but I don''t know if it is good bison practice or not. (aside; I had to find and install the Lenny version of bison to make the autogen diff readable. We should bump this to at least Squeeze in 4.3. I also trimmed the diff to the generated files to just the relevant looking bit -- in particular I trimmed a lot of stuff which appeared to be semi-automated modifications touching the generated files, e.g. the addition of emacs blocks and removal of page breaks ^L)> Other callers to readdata() seem to call exit(1) if readdata() fails.For 4.2 I would be happy with a patch to make this user do the same as a lower risk fix than changing the parser. Ian. diff -r af7143d97fa2 tools/libxl/libxlu_cfg_y.c --- a/tools/libxl/libxlu_cfg_y.c Tue Aug 14 15:59:38 2012 +0100 +++ b/tools/libxl/libxlu_cfg_y.c Wed Aug 15 17:34:25 2012 +0100 @@ -1418,7 +1418,7 @@ yyreduce: { case 4: #line 50 "libxlu_cfg_y.y" - { xlu__cfg_set_store(ctx,(yyvsp[(1) - (3)].string),(yyvsp[(3) - (3)].setting),(yylsp[(3) - (3)]).first_line); ;} + { xlu__cfg_set_store(ctx,(yyvsp[(1) - (3)].string),(yyvsp[(3) - (3)].setting),(yylsp[(3) - (3)]).first_line); (yyvsp[(3) - (3)].setting) = NULL; ;} break; case 10: diff -r af7143d97fa2 tools/libxl/libxlu_cfg_y.y --- a/tools/libxl/libxlu_cfg_y.y Tue Aug 14 15:59:38 2012 +0100 +++ b/tools/libxl/libxlu_cfg_y.y Wed Aug 15 17:34:25 2012 +0100 @@ -47,7 +47,7 @@ file: /* empty */ | file setting -setting: IDENT ''='' value { xlu__cfg_set_store(ctx,$1,$3,@3.first_line); } +setting: IDENT ''='' value { xlu__cfg_set_store(ctx,$1,$3,@3.first_line); $3 = NULL; } endstmt | endstmt | error NEWLINE
Ian Jackson
2012-Aug-23 18:15 UTC
Re: [TESTDAY] xl cpupool-create segfaults if given invalid configuration
Ian Campbell writes ("Re: [Xen-devel] [TESTDAY] xl cpupool-create segfaults
if given invalid configuration"):> This stuff is more of an Ian J thing but I wonder if when we hit the
> syntax error that $$ still refers to the last value parsed, which we
> think we are finished with but actually aren''t? i.e.
we''ve already
> stashed it in the cfg and the reference in $$ is now "stale".
I will look at this tomorrow, but:
> (aside; I had to find and install the Lenny version of bison to make the
> autogen diff readable. We should bump this to at least Squeeze in 4.3. I
> also trimmed the diff to the generated files to just the relevant
> looking bit -- in particular I trimmed a lot of stuff which appeared to
> be semi-automated modifications touching the generated files, e.g. the
> addition of emacs blocks and removal of page breaks ^L)
Perhaps we should do this now. I don''t think there''s any
reason to
fear the squeeze version of bison.
Ian.
Ian Campbell
2012-Aug-24 09:08 UTC
Re: [TESTDAY] xl cpupool-create segfaults if given invalid configuration
On Thu, 2012-08-23 at 19:15 +0100, Ian Jackson wrote:> Ian Campbell writes ("Re: [Xen-devel] [TESTDAY] xl cpupool-create segfaults if given invalid configuration"): > > This stuff is more of an Ian J thing but I wonder if when we hit the > > syntax error that $$ still refers to the last value parsed, which we > > think we are finished with but actually aren''t? i.e. we''ve already > > stashed it in the cfg and the reference in $$ is now "stale". > > I will look at this tomorrow, but: > > > (aside; I had to find and install the Lenny version of bison to make the > > autogen diff readable. We should bump this to at least Squeeze in 4.3. I > > also trimmed the diff to the generated files to just the relevant > > looking bit -- in particular I trimmed a lot of stuff which appeared to > > be semi-automated modifications touching the generated files, e.g. the > > addition of emacs blocks and removal of page breaks ^L) > > Perhaps we should do this now. I don''t think there''s any reason to > fear the squeeze version of bison.I''d be happy with that. When regenerating with Lenny''s bison I got the following spurious changes, no doubt due to some automated tree wide cleanup, which would be nice to dispose of too. diff -r af7143d97fa2 tools/libxl/libxlu_cfg_y.c --- a/tools/libxl/libxlu_cfg_y.c Tue Aug 14 15:59:38 2012 +0100 +++ b/tools/libxl/libxlu_cfg_y.c Fri Aug 24 10:07:28 2012 +0100 @@ -819,7 +819,7 @@ int yydebug; # define YYMAXDEPTH 10000 #endif - + #if YYERROR_VERBOSE @@ -1030,7 +1030,7 @@ yysyntax_error (char *yyresult, int yyst } } #endif /* YYERROR_VERBOSE */ - + /*-----------------------------------------------. | Release the memory associated to this symbol. | @@ -1101,7 +1101,7 @@ yydestruct (yymsg, yytype, yyvaluep, yyl break; } } - + /* Prevent warnings from -Wmissing-prototypes. */ @@ -1689,11 +1689,3 @@ yyreturn: - -/* - * Local variables: - * mode: C - * c-basic-offset: 4 - * indent-tabs-mode: nil - * End: - */ diff -r af7143d97fa2 tools/libxl/libxlu_cfg_y.h --- a/tools/libxl/libxlu_cfg_y.h Tue Aug 14 15:59:38 2012 +0100 +++ b/tools/libxl/libxlu_cfg_y.h Fri Aug 24 10:07:28 2012 +0100 @@ -85,11 +85,3 @@ typedef struct YYLTYPE #endif - -/* - * Local variables: - * mode: C - * c-basic-offset: 4 - * indent-tabs-mode: nil - * End: - */
Ian Jackson
2012-Aug-24 10:46 UTC
Re: [TESTDAY] xl cpupool-create segfaults if given invalid configuration
Ian Campbell writes ("Re: [Xen-devel] [TESTDAY] xl cpupool-create segfaults
if given invalid configuration"):> When regenerating with Lenny''s bison I got the following spurious
> changes, no doubt due to some automated tree wide cleanup, which would
> be nice to dispose of too.
OK. Shall I just cause bison to rerun on my squeeze workstation and
commit the result then ?
Ian.
Ian Campbell
2012-Aug-24 10:49 UTC
Re: [TESTDAY] xl cpupool-create segfaults if given invalid configuration
On Fri, 2012-08-24 at 11:46 +0100, Ian Jackson wrote:> Ian Campbell writes ("Re: [Xen-devel] [TESTDAY] xl cpupool-create segfaults if given invalid configuration"): > > When regenerating with Lenny''s bison I got the following spurious > > changes, no doubt due to some automated tree wide cleanup, which would > > be nice to dispose of too. > > OK. Shall I just cause bison to rerun on my squeeze workstation and > commit the result then ?Please! Ian,
Ian Jackson
2012-Aug-24 11:17 UTC
Re: [TESTDAY] xl cpupool-create segfaults if given invalid configuration
Ian Campbell writes ("Re: [Xen-devel] [TESTDAY] xl cpupool-create segfaults
if given invalid configuration"):> On Fri, 2012-08-24 at 11:46 +0100, Ian Jackson wrote:
> > OK. Shall I just cause bison to rerun on my squeeze workstation and
> > commit the result then ?
>
> Please!
In this context, I found this helpful.
Ian.
# HG changeset patch
# User Ian Jackson <Ian.Jackson@eu.citrix.com>
# Date 1345806920 -3600
# Node ID 7769c3a0511edea42140c6c16608e16195a15182
# Parent 4ca40e0559c33205fb5163b10249a0fd5fda39b9
libxl: provide "make realclean" target
This removes all the autogenerated files.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
diff -r 4ca40e0559c3 -r 7769c3a0511e tools/libxl/Makefile
--- a/tools/libxl/Makefile Thu Aug 23 19:12:28 2012 +0100
+++ b/tools/libxl/Makefile Fri Aug 24 12:15:20 2012 +0100
@@ -212,8 +212,10 @@ clean:
$(RM) -f _*.h *.o *.so* *.a $(CLIENTS) $(DEPS)
$(RM) -f _*.c *.pyc _paths.*.tmp _*.api-for-check
$(RM) -f testidl.c.new testidl.c
-# $(RM) -f $(AUTOSRCS) $(AUTOINCS)
distclean: clean
+realclean: distclean
+ $(RM) -f $(AUTOSRCS) $(AUTOINCS)
+
-include $(DEPS)
Ian Campbell
2012-Aug-24 11:28 UTC
Re: [TESTDAY] xl cpupool-create segfaults if given invalid configuration
On Fri, 2012-08-24 at 12:17 +0100, Ian Jackson wrote:> Ian Campbell writes ("Re: [Xen-devel] [TESTDAY] xl cpupool-create segfaults if given invalid configuration"): > > On Fri, 2012-08-24 at 11:46 +0100, Ian Jackson wrote: > > > OK. Shall I just cause bison to rerun on my squeeze workstation and > > > commit the result then ? > > > > Please! > > In this context, I found this helpful. > > Ian. > > # HG changeset patch > # User Ian Jackson <Ian.Jackson@eu.citrix.com> > # Date 1345806920 -3600 > # Node ID 7769c3a0511edea42140c6c16608e16195a15182 > # Parent 4ca40e0559c33205fb5163b10249a0fd5fda39b9 > libxl: provide "make realclean" target > > This removes all the autogenerated files. > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>> > diff -r 4ca40e0559c3 -r 7769c3a0511e tools/libxl/Makefile > --- a/tools/libxl/Makefile Thu Aug 23 19:12:28 2012 +0100 > +++ b/tools/libxl/Makefile Fri Aug 24 12:15:20 2012 +0100 > @@ -212,8 +212,10 @@ clean: > $(RM) -f _*.h *.o *.so* *.a $(CLIENTS) $(DEPS) > $(RM) -f _*.c *.pyc _paths.*.tmp _*.api-for-check > $(RM) -f testidl.c.new testidl.c > -# $(RM) -f $(AUTOSRCS) $(AUTOINCS) > > distclean: clean > > +realclean: distclean > + $(RM) -f $(AUTOSRCS) $(AUTOINCS) > + > -include $(DEPS)
Ian Jackson
2012-Aug-24 11:40 UTC
Re: [TESTDAY] xl cpupool-create segfaults if given invalid configuration
Ian Campbell writes ("Re: [Xen-devel] [TESTDAY] xl cpupool-create segfaults
if given invalid configuration"):> On Fri, 2012-08-24 at 12:17 +0100, Ian Jackson wrote:
> > Ian Campbell writes ("Re: [Xen-devel] [TESTDAY] xl cpupool-create
segfaults if given invalid configuration"):
> > > On Fri, 2012-08-24 at 11:46 +0100, Ian Jackson wrote:
> > > > OK. Shall I just cause bison to rerun on my squeeze
workstation and
> > > > commit the result then ?
> > >
> > > Please!
Now done.
> > libxl: provide "make realclean" target
> >
> > This removes all the autogenerated files.
> >
> > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
and this also
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian.
Ian Jackson
2012-Aug-30 15:44 UTC
Re: [TESTDAY] xl cpupool-create segfaults if given invalid configuration
Ian Campbell writes ("Re: [Xen-devel] [TESTDAY] xl cpupool-create segfaults
if given invalid configuration"):> I think the issue is the parser has:
> %destructor { xlu__cfg_set_free($$); } value valuelist values
> which frees the current "setting" but does not remove it from the
list
> of settings.
Sorry for this, this is my fault and I have dropped the fix.
> diff -r af7143d97fa2 tools/libxl/libxlu_cfg_y.y
> --- a/tools/libxl/libxlu_cfg_y.y Tue Aug 14 15:59:38 2012 +0100
> +++ b/tools/libxl/libxlu_cfg_y.y Wed Aug 15 17:34:25 2012 +0100
> @@ -47,7 +47,7 @@
> file: /* empty */
> | file setting
>
> -setting: IDENT ''='' value {
xlu__cfg_set_store(ctx,$1,$3,@3.first_line); }
> +setting: IDENT ''='' value {
xlu__cfg_set_store(ctx,$1,$3,@3.first_line); $3 = NULL; }
I don''t think this is correct. It may happen to work with this
version of bison but I don''t think you''re allowed to assign to
$3.
Looking at the code I think this handling of the XLU_ConfigSettings
and flex is all wrong.
Ian.
Ian Campbell
2012-Aug-30 15:52 UTC
Re: [TESTDAY] xl cpupool-create segfaults if given invalid configuration
On Thu, 2012-08-30 at 16:44 +0100, Ian Jackson wrote:> Ian Campbell writes ("Re: [Xen-devel] [TESTDAY] xl cpupool-create segfaults if given invalid configuration"): > > I think the issue is the parser has: > > %destructor { xlu__cfg_set_free($$); } value valuelist values > > which frees the current "setting" but does not remove it from the list > > of settings. > > Sorry for this, this is my fault and I have dropped the fix. > > > diff -r af7143d97fa2 tools/libxl/libxlu_cfg_y.y > > --- a/tools/libxl/libxlu_cfg_y.y Tue Aug 14 15:59:38 2012 +0100 > > +++ b/tools/libxl/libxlu_cfg_y.y Wed Aug 15 17:34:25 2012 +0100 > > @@ -47,7 +47,7 @@ > > file: /* empty */ > > | file setting > > > > -setting: IDENT ''='' value { xlu__cfg_set_store(ctx,$1,$3,@3.first_line); } > > +setting: IDENT ''='' value { xlu__cfg_set_store(ctx,$1,$3,@3.first_line); $3 = NULL; } > > I don''t think this is correct. It may happen to work with this > version of bison but I don''t think you''re allowed to assign to $3.I suspected this might not be ok.> Looking at the code I think this handling of the XLU_ConfigSettings > and flex is all wrong.Does this mean you know what the right fix is and/or a patch is in progress? Ian.
Ian Jackson
2012-Aug-30 16:49 UTC
Re: [TESTDAY] xl cpupool-create segfaults if given invalid configuration
Ian Jackson writes ("Re: [Xen-devel] [TESTDAY] xl cpupool-create segfaults
if given invalid configuration"):> I don''t think this is correct. It may happen to work with this
> version of bison but I don''t think you''re allowed to
assign to $3.
I think this fixes it.
Ian.
From: Ian Jackson <ian.jackson@eu.citrix.com>
Subject: [PATCH] libxl: fix double free on some config parser errors
If libxlu_cfg_y.y encountered a config file error, the code generated
by bison would sometimes _both_ run the %destructor _and_ call
xlu__cfg_set_store for the same XLU_ConfigSetting* semantic value.
The result would be a double free.
This appears to be because of the use of a mid-rule action. There is
some discussion of the problems with destructors and mid-rule action
error handling in "(bison)Mid-Rule Actions". This area is complex and
best avoided.
So fix the bug by abolishing the use of a mid-rule action, which was
in any case not necessary here.
Also while we are there rename the nonterminal rule "setting" to
"assignment", to avoid confusion with the token type
"setting", which
had an identically name in a different namespace. This was especially
confusing because the nonterminal "setting" did not have
"setting" as
the type of its semantic value! (In fact the nonterminal, now called
"assignment", does not have a value so it does not have a value type.)
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/libxlu_cfg_y.c | 120 +++++++++++++++++++++-----------------------
tools/libxl/libxlu_cfg_y.y | 6 +-
2 files changed, 61 insertions(+), 65 deletions(-)
diff --git a/tools/libxl/libxlu_cfg_y.c b/tools/libxl/libxlu_cfg_y.c
index 7d1f6c8..5214386 100644
--- a/tools/libxl/libxlu_cfg_y.c
+++ b/tools/libxl/libxlu_cfg_y.c
@@ -380,11 +380,11 @@ union yyalloc
/* YYNTOKENS -- Number of terminals. */
#define YYNTOKENS 12
/* YYNNTS -- Number of nonterminals. */
-#define YYNNTS 10
+#define YYNNTS 9
/* YYNRULES -- Number of rules. */
-#define YYNRULES 20
+#define YYNRULES 19
/* YYNRULES -- Number of states. */
-#define YYNSTATES 29
+#define YYNSTATES 28
/* YYTRANSLATE(YYLEX) -- Bison symbol number corresponding to YYLEX. */
#define YYUNDEFTOK 2
@@ -430,28 +430,26 @@ static const yytype_uint8 yytranslate[] YYRHS. */
static const yytype_uint8 yyprhs[] {
- 0, 0, 3, 4, 7, 8, 14, 16, 19, 21,
- 23, 25, 30, 32, 34, 35, 37, 41, 44, 50,
- 51
+ 0, 0, 3, 4, 7, 12, 14, 17, 19, 21,
+ 23, 28, 30, 32, 33, 35, 39, 42, 48, 49
};
/* YYRHS -- A `-1''-separated list of the rules'' RHS. */
static const yytype_int8 yyrhs[] {
- 13, 0, -1, -1, 13, 14, -1, -1, 3, 7,
- 17, 15, 16, -1, 16, -1, 1, 6, -1, 6,
- -1, 8, -1, 18, -1, 9, 21, 19, 10, -1,
- 4, -1, 5, -1, -1, 20, -1, 20, 11, 21,
- -1, 18, 21, -1, 20, 11, 21, 18, 21, -1,
- -1, 21, 6, -1
+ 13, 0, -1, -1, 13, 14, -1, 3, 7, 16,
+ 15, -1, 15, -1, 1, 6, -1, 6, -1, 8,
+ -1, 17, -1, 9, 20, 18, 10, -1, 4, -1,
+ 5, -1, -1, 19, -1, 19, 11, 20, -1, 17,
+ 20, -1, 19, 11, 20, 17, 20, -1, -1, 20,
+ 6, -1
};
/* YYRLINE[YYN] -- source line where rule number YYN was defined. */
static const yytype_uint8 yyrline[] {
- 0, 47, 47, 48, 50, 50, 52, 53, 55, 56,
- 58, 59, 61, 62, 64, 65, 66, 68, 69, 71,
- 73
+ 0, 47, 47, 48, 50, 52, 53, 55, 56, 58,
+ 59, 61, 62, 64, 65, 66, 68, 69, 71, 73
};
#endif
@@ -461,7 +459,7 @@ static const yytype_uint8 yyrline[] static const char
*const yytname[] {
"$end", "error", "$undefined",
"IDENT", "STRING", "NUMBER", "NEWLINE",
- "''=''", "'';''",
"''[''", "'']''",
"'',''", "$accept", "file",
"setting", "$@1",
+ "''=''", "'';''",
"''[''", "'']''",
"'',''", "$accept", "file",
"assignment",
"endstmt", "value", "atom",
"valuelist", "values", "nlok", 0
};
#endif
@@ -479,17 +477,15 @@ static const yytype_uint16 yytoknum[] /* YYR1[YYN] --
Symbol number of symbol that rule YYN derives. */
static const yytype_uint8 yyr1[] {
- 0, 12, 13, 13, 15, 14, 14, 14, 16, 16,
- 17, 17, 18, 18, 19, 19, 19, 20, 20, 21,
- 21
+ 0, 12, 13, 13, 14, 14, 14, 15, 15, 16,
+ 16, 17, 17, 18, 18, 18, 19, 19, 20, 20
};
/* YYR2[YYN] -- Number of symbols composing right hand side of rule YYN. */
static const yytype_uint8 yyr2[] {
- 0, 2, 0, 2, 0, 5, 1, 2, 1, 1,
- 1, 4, 1, 1, 0, 1, 3, 2, 5, 0,
- 2
+ 0, 2, 0, 2, 4, 1, 2, 1, 1, 1,
+ 4, 1, 1, 0, 1, 3, 2, 5, 0, 2
};
/* YYDEFACT[STATE-NAME] -- Default rule to reduce with in state
@@ -497,15 +493,15 @@ static const yytype_uint8 yyr2[] means the default is
an error. */
static const yytype_uint8 yydefact[] {
- 2, 0, 1, 0, 0, 8, 9, 3, 6, 7,
- 0, 12, 13, 19, 4, 10, 14, 0, 20, 19,
- 0, 15, 5, 17, 11, 19, 16, 19, 18
+ 2, 0, 1, 0, 0, 7, 8, 3, 5, 6,
+ 0, 11, 12, 18, 0, 9, 13, 4, 19, 18,
+ 0, 14, 16, 10, 18, 15, 18, 17
};
/* YYDEFGOTO[NTERM-NUM]. */
static const yytype_int8 yydefgoto[] {
- -1, 1, 7, 17, 8, 14, 15, 20, 21, 16
+ -1, 1, 7, 8, 14, 15, 20, 21, 16
};
/* YYPACT[STATE-NUM] -- Index in YYTABLE of the portion describing
@@ -513,15 +509,15 @@ static const yytype_int8 yydefgoto[] #define YYPACT_NINF
-17
static const yytype_int8 yypact[] {
- -17, 1, -17, -3, 5, -17, -17, -17, -17, -17,
- 10, -17, -17, -17, -17, -17, 12, 0, -17, -17,
- 11, 9, -17, 16, -17, -17, 12, -17, 16
+ -17, 2, -17, -5, -3, -17, -17, -17, -17, -17,
+ 10, -17, -17, -17, 14, -17, 12, -17, -17, -17,
+ 11, -4, 6, -17, -17, 12, -17, 6
};
/* YYPGOTO[NTERM-NUM]. */
static const yytype_int8 yypgoto[] {
- -17, -17, -17, -17, 6, -17, -16, -17, -17, -14
+ -17, -17, -17, 9, -17, -16, -17, -17, -13
};
/* YYTABLE[YYPACT[STATE-NUM]]. What to do in state STATE-NUM. If
@@ -531,25 +527,25 @@ static const yytype_int8 yypgoto[] #define YYTABLE_NINF
-1
static const yytype_uint8 yytable[] {
- 19, 2, 3, 9, 4, 23, 5, 5, 6, 6,
- 27, 26, 10, 28, 11, 12, 11, 12, 18, 13,
- 25, 24, 18, 22
+ 19, 9, 2, 3, 10, 4, 22, 24, 5, 26,
+ 6, 25, 18, 27, 11, 12, 11, 12, 18, 13,
+ 5, 23, 6, 17
};
static const yytype_uint8 yycheck[] {
- 16, 0, 1, 6, 3, 19, 6, 6, 8, 8,
- 26, 25, 7, 27, 4, 5, 4, 5, 6, 9,
- 11, 10, 6, 17
+ 16, 6, 0, 1, 7, 3, 19, 11, 6, 25,
+ 8, 24, 6, 26, 4, 5, 4, 5, 6, 9,
+ 6, 10, 8, 14
};
/* YYSTOS[STATE-NUM] -- The (internal number of the) accessing
symbol of state STATE-NUM. */
static const yytype_uint8 yystos[] {
- 0, 13, 0, 1, 3, 6, 8, 14, 16, 6,
- 7, 4, 5, 9, 17, 18, 21, 15, 6, 18,
- 19, 20, 16, 21, 10, 11, 21, 18, 21
+ 0, 13, 0, 1, 3, 6, 8, 14, 15, 6,
+ 7, 4, 5, 9, 16, 17, 20, 15, 6, 17,
+ 18, 19, 20, 10, 11, 20, 17, 20
};
#define yyerrok (yyerrstatus = 0)
@@ -1081,7 +1077,7 @@ yydestruct (yymsg, yytype, yyvaluep, yylocationp, ctx)
{ free((yyvaluep->string)); };
/* Line 1000 of yacc.c */
-#line 1085 "libxlu_cfg_y.c"
+#line 1081 "libxlu_cfg_y.c"
break;
case 4: /* "STRING" */
@@ -1090,7 +1086,7 @@ yydestruct (yymsg, yytype, yyvaluep, yylocationp, ctx)
{ free((yyvaluep->string)); };
/* Line 1000 of yacc.c */
-#line 1094 "libxlu_cfg_y.c"
+#line 1090 "libxlu_cfg_y.c"
break;
case 5: /* "NUMBER" */
@@ -1099,43 +1095,43 @@ yydestruct (yymsg, yytype, yyvaluep, yylocationp, ctx)
{ free((yyvaluep->string)); };
/* Line 1000 of yacc.c */
-#line 1103 "libxlu_cfg_y.c"
+#line 1099 "libxlu_cfg_y.c"
break;
- case 17: /* "value" */
+ case 16: /* "value" */
/* Line 1000 of yacc.c */
#line 43 "libxlu_cfg_y.y"
{ xlu__cfg_set_free((yyvaluep->setting)); };
/* Line 1000 of yacc.c */
-#line 1112 "libxlu_cfg_y.c"
+#line 1108 "libxlu_cfg_y.c"
break;
- case 18: /* "atom" */
+ case 17: /* "atom" */
/* Line 1000 of yacc.c */
#line 40 "libxlu_cfg_y.y"
{ free((yyvaluep->string)); };
/* Line 1000 of yacc.c */
-#line 1121 "libxlu_cfg_y.c"
+#line 1117 "libxlu_cfg_y.c"
break;
- case 19: /* "valuelist" */
+ case 18: /* "valuelist" */
/* Line 1000 of yacc.c */
#line 43 "libxlu_cfg_y.y"
{ xlu__cfg_set_free((yyvaluep->setting)); };
/* Line 1000 of yacc.c */
-#line 1130 "libxlu_cfg_y.c"
+#line 1126 "libxlu_cfg_y.c"
break;
- case 20: /* "values" */
+ case 19: /* "values" */
/* Line 1000 of yacc.c */
#line 43 "libxlu_cfg_y.y"
{ xlu__cfg_set_free((yyvaluep->setting)); };
/* Line 1000 of yacc.c */
-#line 1139 "libxlu_cfg_y.c"
+#line 1135 "libxlu_cfg_y.c"
break;
default:
@@ -1466,67 +1462,67 @@ yyreduce:
case 4:
/* Line 1455 of yacc.c */
-#line 50 "libxlu_cfg_y.y"
- { xlu__cfg_set_store(ctx,(yyvsp[(1) - (3)].string),(yyvsp[(3) -
(3)].setting),(yylsp[(3) - (3)]).first_line); ;}
+#line 51 "libxlu_cfg_y.y"
+ { xlu__cfg_set_store(ctx,(yyvsp[(1) - (4)].string),(yyvsp[(3) -
(4)].setting),(yylsp[(3) - (4)]).first_line); ;}
break;
- case 10:
+ case 9:
/* Line 1455 of yacc.c */
#line 58 "libxlu_cfg_y.y"
{ (yyval.setting)= xlu__cfg_set_mk(ctx,1,(yyvsp[(1) - (1)].string)); ;}
break;
- case 11:
+ case 10:
/* Line 1455 of yacc.c */
#line 59 "libxlu_cfg_y.y"
{ (yyval.setting)= (yyvsp[(3) - (4)].setting); ;}
break;
- case 12:
+ case 11:
/* Line 1455 of yacc.c */
#line 61 "libxlu_cfg_y.y"
{ (yyval.string)= (yyvsp[(1) - (1)].string); ;}
break;
- case 13:
+ case 12:
/* Line 1455 of yacc.c */
#line 62 "libxlu_cfg_y.y"
{ (yyval.string)= (yyvsp[(1) - (1)].string); ;}
break;
- case 14:
+ case 13:
/* Line 1455 of yacc.c */
#line 64 "libxlu_cfg_y.y"
{ (yyval.setting)= xlu__cfg_set_mk(ctx,0,0); ;}
break;
- case 15:
+ case 14:
/* Line 1455 of yacc.c */
#line 65 "libxlu_cfg_y.y"
{ (yyval.setting)= (yyvsp[(1) - (1)].setting); ;}
break;
- case 16:
+ case 15:
/* Line 1455 of yacc.c */
#line 66 "libxlu_cfg_y.y"
{ (yyval.setting)= (yyvsp[(1) - (3)].setting); ;}
break;
- case 17:
+ case 16:
/* Line 1455 of yacc.c */
#line 68 "libxlu_cfg_y.y"
{ (yyval.setting)= xlu__cfg_set_mk(ctx,2,(yyvsp[(1) - (2)].string)); ;}
break;
- case 18:
+ case 17:
/* Line 1455 of yacc.c */
#line 69 "libxlu_cfg_y.y"
@@ -1536,7 +1532,7 @@ yyreduce:
/* Line 1455 of yacc.c */
-#line 1540 "libxlu_cfg_y.c"
+#line 1536 "libxlu_cfg_y.c"
default: break;
}
YY_SYMBOL_PRINT ("-> $$ =", yyr1[yyn], &yyval, &yyloc);
diff --git a/tools/libxl/libxlu_cfg_y.y b/tools/libxl/libxlu_cfg_y.y
index f0a0559..29aedca 100644
--- a/tools/libxl/libxlu_cfg_y.y
+++ b/tools/libxl/libxlu_cfg_y.y
@@ -45,10 +45,10 @@
%%
file: /* empty */
- | file setting
+ | file assignment
-setting: IDENT ''='' value {
xlu__cfg_set_store(ctx,$1,$3,@3.first_line); }
- endstmt
+assignment: IDENT ''='' value endstmt
+ { xlu__cfg_set_store(ctx,$1,$3,@3.first_line); }
| endstmt
| error NEWLINE
--
tg: (5babc64..) t/xen/xl.cfg.mem-fix (depends on: base)
Ian Campbell
2012-Aug-31 11:15 UTC
Re: [TESTDAY] xl cpupool-create segfaults if given invalid configuration
On Thu, 2012-08-30 at 17:49 +0100, Ian Jackson wrote:> Ian Jackson writes ("Re: [Xen-devel] [TESTDAY] xl cpupool-create segfaults if given invalid configuration"): > > I don''t think this is correct. It may happen to work with this > > version of bison but I don''t think you''re allowed to assign to $3. > > I think this fixes it.It works for me.> > Ian. > > From: Ian Jackson <ian.jackson@eu.citrix.com> > Subject: [PATCH] libxl: fix double free on some config parser errors > > If libxlu_cfg_y.y encountered a config file error, the code generated > by bison would sometimes _both_ run the %destructor _and_ call > xlu__cfg_set_store for the same XLU_ConfigSetting* semantic value. > The result would be a double free. > > This appears to be because of the use of a mid-rule action. There is > some discussion of the problems with destructors and mid-rule action > error handling in "(bison)Mid-Rule Actions". This area is complex and > best avoided. > > So fix the bug by abolishing the use of a mid-rule action, which was > in any case not necessary here. > > Also while we are there rename the nonterminal rule "setting" to > "assignment", to avoid confusion with the token type "setting", which > had an identically name in a different namespace. This was especially > confusing because the nonterminal "setting" did not have "setting" as > the type of its semantic value! (In fact the nonterminal, now called > "assignment", does not have a value so it does not have a value type.) > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>Acked-by: Ian Campbell <ian.campbell@citrix.com> I shall apply in a moment, thanks.