Re: [PATCH] grep: correctly initialize help-all option

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2323 bytes --]

On Fri, Apr 10, 2015 at 06:34:29PM +0200, René Scharfe wrote:
> Am 10.04.2015 um 07:22 schrieb Patrick Steinhardt:
> > On Thu, Apr 09, 2015 at 11:55:01PM +0200, René Scharfe wrote:
> >> Am 09.04.2015 um 15:41 schrieb Patrick Steinhardt:
> >>> ---
> >>>    builtin/grep.c | 2 +-
> >>>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/builtin/grep.c b/builtin/grep.c
> >>> index abc4400..c0bf005 100644
> >>> --- a/builtin/grep.c
> >>> +++ b/builtin/grep.c
> >>> @@ -738,7 +738,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
> >>>    			PARSE_OPT_OPTARG, NULL, (intptr_t)default_pager },
> >>>    		OPT_BOOL(0, "ext-grep", &external_grep_allowed__ignored,
> >>>    			 N_("allow calling of grep(1) (ignored by this build)")),
> >>> -		{ OPTION_CALLBACK, 0, "help-all", &options, NULL, N_("show usage"),
> >>> +		{ OPTION_CALLBACK, 0, "help-all", &opt, NULL, N_("show usage"),
> >>>    		  PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, help_callback },
> >>>    		OPT_END()
> >>>    	};
> >>
> >> help_callback() returns -1 immediately, IOW the value pointer is never
> >> used anyway.  So why does your change make a difference?  *puzzled*
> >>
> >> We could pass NULL instead, as in builtin/show-ref.c, which would make
> >> it clear that the pointer is just a dummy.
> >
> > Changed in v2, as well.
> 
> Thank you.  I should really re-fetch from Gmane before finishing an 
> interrupted reply..
> 
> > In general the change won't make any difference when running the
> > command. But as said in the commit message it caused gcc (gcc
> > version 4.8.3 (Gentoo Hardened 4.8.3 p1.1, pie-0.5.9), ARMv7 HF)
> > to segfault when &options was passed in as value. Even though
> > this is probably an error in gcc we can easily work around it by
> > doing the Right Thing here.
> 
> OK, so does it crash on this one-liner as well?
> 
> 	struct t {void *p;} s = {&s};
> 
> Or on this?
> 
> 	void *p = &p;
> 
> If yes then the author of the hardening feature might be interested in 
> this fact.
> 
> René

No, the segfault is not triggered by this. I've also tried to use
the exact arguments to gcc that are used when compiling grep.c,
but to no avail. I'll maybe try to reproduce this with a minimal
testcase next week.

Regards
Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]