Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/1] configure: use appropriate code fragment for -fstack-protector checks

[Rodrigo Rebello]

Markus,

2015-11-12 11:29 GMT-02:00 Markus Armbruster <address@hidden>:
> Rodrigo Rebello <address@hidden> writes:
>
>> Markus,
>>
>> 2015-11-12 6:41 GMT-02:00 Markus Armbruster <address@hidden>:
>>> Rodrigo Rebello <address@hidden> writes:
>>>
>>>> The check for stack-protector support consisted in compiling and linking
>>>> the test program below (output by function write_c_skeleton()) with the
>>>> compiler flag -fstack-protector-strong first and then with
>>>> -fstack-protector-all if the first one failed to work:
>>>>
>>>>   int main(void) { return 0; }
>>>>
>>>> This caused false positives when using certain toolchains in which the
>>>> compiler accepted -fstack-protector-strong but no support was provided
>>>> by the C library, since for this stack-protector variant the compiler
>>>> emits canary code only for functions that meet specific conditions
>>>> (local arrays, memory references to local variables, etc.) and the code
>>>> fragment under test included none of them (hence no stack protection
>>>> code generated, no link failure).
>>>>
>>>> This fix changes the test program used for -fstack-protector checks to
>>>> include a function that meets conditions which cause the compiler to
>>>> generate canary code in all variants.
>>>>
>>>> Signed-off-by: Rodrigo Rebello <address@hidden>
>>>> ---
>>>>  configure | 18 ++++++++++++++++++
>>>>  1 file changed, 18 insertions(+)
>>>>
>>>> diff --git a/configure b/configure
>>>> index 46fd8bd..c3d9592 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -1486,6 +1486,24 @@ for flag in $gcc_flags; do
>>>>  done
>>>>
>>>>  if test "$stack_protector" != "no"; then
>>>> +  cat > $TMPC << EOF
>>>> +void foo(const char *c);
>>>> +
>>>> +void foo(const char *c)
>>>> +{
>>>> +    char arr[64], *p;
>>>> +    for (p = arr; *c; c++, p++) {
>>>> +        *p = *c;
>>>> +    }
>>>> +}
>>>> +
>>>> +int main(void)
>>>> +{
>>>> +    char c[] = "";
>>>> +    foo(c);
>>>
>>> Why not simply foo("")?
>>>
>>> Could the optimizer optimize away the pattern that triggers the canary?
>>>
>>> To protect against that possibility, we could use
>>>
>>> int main(int argc, char *argv[])
>>> {
>>>     foo(argv[0]);
>>> }
>>>
>>
>> You're right, this can be made simpler and the version you suggested
>> works as well (even if I force different optimization levels in
>> QEMU_CFLAGS).
>> In fact, I've come up with an even simpler version which does not
>> involve a "foo" function:
>>
>> int main(int argc, char *argv[])
>> {
>>     char arr[64], *p = arr, *c = argv[0];
>>     while (*c) {
>>         *p++ = *c++;
>>     }
>>     return 0;
>> }
>>
>> What do you think of this one?
>
> There's the theoretical possibility that the compiler treats main()
> specially.
>
> But then there's also the even more theoretical possibility that the
> compiler inlines foo() into main() at link time, throws away foo(), and
> treats main() specially.
>
> We can worry about theoretical possibilities all day long.  Instead,
> please use your judgement to pick something that works now and looks
> reasonably robust to you.

Ok, then. I'll stick to the last version I proposed and send a new patch.

Regards,
Rodrigo