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

["René Scharfe" <l.s.r@web.de>]

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é