[Buildroot] [PATCH v2 1/1] qemu: add patch to fix SSP support detection

[Buildroot] [PATCH v2 1/1] qemu: add patch to fix SSP support detection

[Rodrigo Rebello]

The QEMU configure script incorrectly assumes SSP is supported by the
toolchain in some cases where the compiler accepts -fstack-protector-*
flags but the C library does not provide the necessary __stack_chk_*()
functions.

Even though a full compile and link test is performed by the script,
this is done with a code fragment which does not actually meet any of
the conditions required to cause the compiler to emit canary code when
the -fstack-protector-strong variant is used. As no compile or link
failure occurs in this case, a false positive is generated and a
subsequent error is seen when the probe for pthreads is performed.

The fix consists in patching the configure script to use an appropriate
test program for the SSP support checks.

Fixes:

  http://autobuild.buildroot.net/results/efb/efbb4e940543894b8745bb405478a096c90a5ae2/
  http://autobuild.buildroot.net/results/32d/32d6d984febad2dee1f0d31c5fa0aea823297096/
  http://autobuild.buildroot.net/results/aa6/aa6e71c957fb6f07e7bded35a8e47be4dadd042c/
  ...and many others.

Signed-off-by: Rodrigo Rebello <rprebello@gmail.com>
---
Changes v1 -> v2:
  - Patch the configure script instead of force disable SSP detection
    (Arnout Vandecappelle).
---
 ...se-appropriate-code-fragment-for-fstack-p.patch | 65 ++++++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 package/qemu/0001-configure-use-appropriate-code-fragment-for-fstack-p.patch

diff --git a/package/qemu/0001-configure-use-appropriate-code-fragment-for-fstack-p.patch b/package/qemu/0001-configure-use-appropriate-code-fragment-for-fstack-p.patch
new file mode 100644
index 0000000..5eee141
--- /dev/null
+++ b/package/qemu/0001-configure-use-appropriate-code-fragment-for-fstack-p.patch
@@ -0,0 +1,65 @@
+From 83897ad507f8bb332000304b96d36c109c19bfad Mon Sep 17 00:00:00 2001
+From: Rodrigo Rebello <rprebello@gmail.com>
+Date: Wed, 11 Nov 2015 18:39:24 -0200
+Subject: [PATCH 1/1] configure: use appropriate code fragment for
+ -fstack-protector checks
+Cc: qemu-trivial at nongnu.org
+
+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.
+
+Upstream status: sent
+
+Signed-off-by: Rodrigo Rebello <rprebello@gmail.com>
+---
+ configure | 18 ++++++++++++++++++
+ 1 file changed, 18 insertions(+)
+
+diff --git a/configure b/configure
+index cd219d8..a6f4101 100755
+--- a/configure
++++ b/configure
+@@ -1471,6 +1471,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);
++    return 0;
++}
++EOF
+   gcc_flags="-fstack-protector-strong -fstack-protector-all"
+   sp_on=0
+   for flag in $gcc_flags; do
+-- 
+2.1.4
+
-- 
2.1.4

[Buildroot] [PATCH v2 1/1] qemu: add patch to fix SSP support detection

[Arnout Vandecappelle]

On 11-11-15 23:18, Rodrigo Rebello wrote:
> The QEMU configure script incorrectly assumes SSP is supported by the
> toolchain in some cases where the compiler accepts -fstack-protector-*
> flags but the C library does not provide the necessary __stack_chk_*()
> functions.
> 
> Even though a full compile and link test is performed by the script,
> this is done with a code fragment which does not actually meet any of
> the conditions required to cause the compiler to emit canary code when
> the -fstack-protector-strong variant is used. As no compile or link
> failure occurs in this case, a false positive is generated and a
> subsequent error is seen when the probe for pthreads is performed.
> 
> The fix consists in patching the configure script to use an appropriate
> test program for the SSP support checks.
> 
> Fixes:
> 
>   http://autobuild.buildroot.net/results/efb/efbb4e940543894b8745bb405478a096c90a5ae2/
>   http://autobuild.buildroot.net/results/32d/32d6d984febad2dee1f0d31c5fa0aea823297096/
>   http://autobuild.buildroot.net/results/aa6/aa6e71c957fb6f07e7bded35a8e47be4dadd042c/
>   ...and many others.
> 
> Signed-off-by: Rodrigo Rebello <rprebello@gmail.com>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 But let's wait a day or see for upstream to comment on the patch. I have one nit:

> ---
> Changes v1 -> v2:
>   - Patch the configure script instead of force disable SSP detection
>     (Arnout Vandecappelle).
> ---
>  ...se-appropriate-code-fragment-for-fstack-p.patch | 65 ++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 package/qemu/0001-configure-use-appropriate-code-fragment-for-fstack-p.patch
> 
> diff --git a/package/qemu/0001-configure-use-appropriate-code-fragment-for-fstack-p.patch b/package/qemu/0001-configure-use-appropriate-code-fragment-for-fstack-p.patch
> new file mode 100644
> index 0000000..5eee141
> --- /dev/null
> +++ b/package/qemu/0001-configure-use-appropriate-code-fragment-for-fstack-p.patch
> @@ -0,0 +1,65 @@
> +From 83897ad507f8bb332000304b96d36c109c19bfad Mon Sep 17 00:00:00 2001
> +From: Rodrigo Rebello <rprebello@gmail.com>
> +Date: Wed, 11 Nov 2015 18:39:24 -0200
> +Subject: [PATCH 1/1] configure: use appropriate code fragment for
> + -fstack-protector checks
> +Cc: qemu-trivial at nongnu.org
> +
> +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.
> +
> +Upstream status: sent
> +
> +Signed-off-by: Rodrigo Rebello <rprebello@gmail.com>
> +---
> + configure | 18 ++++++++++++++++++
> + 1 file changed, 18 insertions(+)
> +
> +diff --git a/configure b/configure
> +index cd219d8..a6f4101 100755
> +--- a/configure
> ++++ b/configure
> +@@ -1471,6 +1471,24 @@ for flag in $gcc_flags; do
> + done
> + 
> + if test "$stack_protector" != "no"; then
> ++  cat > $TMPC << EOF
> ++void foo(const char *c);

 This declaration is unnecessary.


 Regards,
 Arnout

> ++
> ++void foo(const char *c)
> ++{
> ++    char arr[64], *p;
> ++    for (p = arr; *c; c++, p++) {
> ++        *p = *c;
> ++    }
> ++}
> ++
> ++int main(void)
> ++{
> ++    char c[] = "";
> ++    foo(c);
> ++    return 0;
> ++}
> ++EOF
> +   gcc_flags="-fstack-protector-strong -fstack-protector-all"
> +   sp_on=0
> +   for flag in $gcc_flags; do
> +-- 
> +2.1.4
> +
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2 1/1] qemu: add patch to fix SSP support detection

[Rodrigo Rebello]

Arnout,

2015-11-11 21:30 GMT-02:00 Arnout Vandecappelle <arnout@mind.be>:
> On 11-11-15 23:18, Rodrigo Rebello wrote:
>> The QEMU configure script incorrectly assumes SSP is supported by the
>> toolchain in some cases where the compiler accepts -fstack-protector-*
>> flags but the C library does not provide the necessary __stack_chk_*()
>> functions.
>>
>> Even though a full compile and link test is performed by the script,
>> this is done with a code fragment which does not actually meet any of
>> the conditions required to cause the compiler to emit canary code when
>> the -fstack-protector-strong variant is used. As no compile or link
>> failure occurs in this case, a false positive is generated and a
>> subsequent error is seen when the probe for pthreads is performed.
>>
>> The fix consists in patching the configure script to use an appropriate
>> test program for the SSP support checks.
>>
>> Fixes:
>>
>>   http://autobuild.buildroot.net/results/efb/efbb4e940543894b8745bb405478a096c90a5ae2/
>>   http://autobuild.buildroot.net/results/32d/32d6d984febad2dee1f0d31c5fa0aea823297096/
>>   http://autobuild.buildroot.net/results/aa6/aa6e71c957fb6f07e7bded35a8e47be4dadd042c/
>>   ...and many others.
>>
>> Signed-off-by: Rodrigo Rebello <rprebello@gmail.com>
>
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>
>  But let's wait a day or see for upstream to comment on the patch. I have one nit:
>
>> ---
>> Changes v1 -> v2:
>>   - Patch the configure script instead of force disable SSP detection
>>     (Arnout Vandecappelle).
>> ---
>>  ...se-appropriate-code-fragment-for-fstack-p.patch | 65 ++++++++++++++++++++++
>>  1 file changed, 65 insertions(+)
>>  create mode 100644 package/qemu/0001-configure-use-appropriate-code-fragment-for-fstack-p.patch
>>
>> diff --git a/package/qemu/0001-configure-use-appropriate-code-fragment-for-fstack-p.patch b/package/qemu/0001-configure-use-appropriate-code-fragment-for-fstack-p.patch
>> new file mode 100644
>> index 0000000..5eee141
>> --- /dev/null
>> +++ b/package/qemu/0001-configure-use-appropriate-code-fragment-for-fstack-p.patch
>> @@ -0,0 +1,65 @@
>> +From 83897ad507f8bb332000304b96d36c109c19bfad Mon Sep 17 00:00:00 2001
>> +From: Rodrigo Rebello <rprebello@gmail.com>
>> +Date: Wed, 11 Nov 2015 18:39:24 -0200
>> +Subject: [PATCH 1/1] configure: use appropriate code fragment for
>> + -fstack-protector checks
>> +Cc: qemu-trivial at nongnu.org
>> +
>> +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.
>> +
>> +Upstream status: sent
>> +
>> +Signed-off-by: Rodrigo Rebello <rprebello@gmail.com>
>> +---
>> + configure | 18 ++++++++++++++++++
>> + 1 file changed, 18 insertions(+)
>> +
>> +diff --git a/configure b/configure
>> +index cd219d8..a6f4101 100755
>> +--- a/configure
>> ++++ b/configure
>> +@@ -1471,6 +1471,24 @@ for flag in $gcc_flags; do
>> + done
>> +
>> + if test "$stack_protector" != "no"; then
>> ++  cat > $TMPC << EOF
>> ++void foo(const char *c);
>
>  This declaration is unnecessary.
>

Actually it is necessary, otherwise compiling the test code fails with:

config-temp/qemu-conf.c:3:6: error: no previous prototype for ?foo?
[-Werror=missing-prototypes]
 void foo(const char *c)
      ^

Because in configure, line 410, -Wmissing-prototypes is added to
QEMU_FLAGS (used in every compile test), and tests are run with
-Werror by default, unless --disable-werror is passed to the configure
script.

>
>  Regards,
>  Arnout
>
>> ++
>> ++void foo(const char *c)
>> ++{
>> ++    char arr[64], *p;
>> ++    for (p = arr; *c; c++, p++) {
>> ++        *p = *c;
>> ++    }
>> ++}
>> ++
>> ++int main(void)
>> ++{
>> ++    char c[] = "";
>> ++    foo(c);
>> ++    return 0;
>> ++}
>> ++EOF
>> +   gcc_flags="-fstack-protector-strong -fstack-protector-all"
>> +   sp_on=0
>> +   for flag in $gcc_flags; do
>> +--
>> +2.1.4
>> +
>>
>
>
> --
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

Regards,
Rodrigo

[Buildroot] [PATCH v2 1/1] qemu: add patch to fix SSP support detection

[Arnout Vandecappelle]

On 12-11-15 00:49, Rodrigo Rebello wrote:
> Arnout,
> 
> 2015-11-11 21:30 GMT-02:00 Arnout Vandecappelle <arnout@mind.be>:
>> On 11-11-15 23:18, Rodrigo Rebello wrote:
[snip]
>>> +diff --git a/configure b/configure
>>> +index cd219d8..a6f4101 100755
>>> +--- a/configure
>>> ++++ b/configure
>>> +@@ -1471,6 +1471,24 @@ for flag in $gcc_flags; do
>>> + done
>>> +
>>> + if test "$stack_protector" != "no"; then
>>> ++  cat > $TMPC << EOF
>>> ++void foo(const char *c);
>>
>>  This declaration is unnecessary.
>>
> 
> Actually it is necessary, otherwise compiling the test code fails with:
> 
> config-temp/qemu-conf.c:3:6: error: no previous prototype for ?foo?
> [-Werror=missing-prototypes]
>  void foo(const char *c)
>       ^
> 
> Because in configure, line 410, -Wmissing-prototypes is added to
> QEMU_FLAGS (used in every compile test), and tests are run with
> -Werror by default, unless --disable-werror is passed to the configure
> script.

 Right. And I guess declaring it static means that the whole function could be
elided so still no stack protection.

 I wonder what would happen if LTO were enabled, since in that case the function
could be elided again...

 To solve this fundamentally in buildroot, perhaps we should patch the gcc
wrapper to fail when -fstack-protector-* is passed when SSP is not enabled.

 Regards,
 Arnout

-- 
Arnout Vandecappelle      arnout dot vandecappelle at essensium dot com
Senior Embedded Software Architect . . . . . . +32-478-010353 (mobile)
Essensium, Mind division . . . . . . . . . . . . . . http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium . . . . . BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2 1/1] qemu: add patch to fix SSP support detection

[Rodrigo Rebello]

Arnout, All

2015-11-12 6:19 GMT-02:00 Arnout Vandecappelle <arnout@mind.be>:
>
>
> On 12-11-15 00:49, Rodrigo Rebello wrote:
>> Arnout,
>>
>> 2015-11-11 21:30 GMT-02:00 Arnout Vandecappelle <arnout@mind.be>:
>>> On 11-11-15 23:18, Rodrigo Rebello wrote:
> [snip]
>>>> +diff --git a/configure b/configure
>>>> +index cd219d8..a6f4101 100755
>>>> +--- a/configure
>>>> ++++ b/configure
>>>> +@@ -1471,6 +1471,24 @@ for flag in $gcc_flags; do
>>>> + done
>>>> +
>>>> + if test "$stack_protector" != "no"; then
>>>> ++  cat > $TMPC << EOF
>>>> ++void foo(const char *c);
>>>
>>>  This declaration is unnecessary.
>>>
>>
>> Actually it is necessary, otherwise compiling the test code fails with:
>>
>> config-temp/qemu-conf.c:3:6: error: no previous prototype for ?foo?
>> [-Werror=missing-prototypes]
>>  void foo(const char *c)
>>       ^
>>
>> Because in configure, line 410, -Wmissing-prototypes is added to
>> QEMU_FLAGS (used in every compile test), and tests are run with
>> -Werror by default, unless --disable-werror is passed to the configure
>> script.
>
>  Right. And I guess declaring it static means that the whole function could be
> elided so still no stack protection.
>
>  I wonder what would happen if LTO were enabled, since in that case the function
> could be elided again...
>

Indeed, I've run a test with LTO enabled and no stack protection code
gets emitted due to the optimizations. Finding a test code fragment
that works for all possibilities isn't so simple after all.

Maybe we should stick to the original v1 of this patch which disables
SSP detection when it's known to be unsupported in the toolchain? Far
from perfect, but at least it's more robust.

>  To solve this fundamentally in buildroot, perhaps we should patch the gcc
> wrapper to fail when -fstack-protector-* is passed when SSP is not enabled.
>

Yes, that would be nice since it would serve as a solution to many
other similar SSP detection failure cases. Perhaps I should look into
that? Or wait until more people provide their views on this?

Regards,
Rodrigo

>  Regards,
>  Arnout
>
> --
> Arnout Vandecappelle      arnout dot vandecappelle at essensium dot com
> Senior Embedded Software Architect . . . . . . +32-478-010353 (mobile)
> Essensium, Mind division . . . . . . . . . . . . . . http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium . . . . . BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
>

[Buildroot] [PATCH v2 1/1] qemu: add patch to fix SSP support detection

[Arnout Vandecappelle]

On 12-11-15 16:04, Rodrigo Rebello wrote:
> Arnout, All
> 
> 2015-11-12 6:19 GMT-02:00 Arnout Vandecappelle <arnout@mind.be>:
>>
>>
>> On 12-11-15 00:49, Rodrigo Rebello wrote:
>>> Arnout,
>>>
>>> 2015-11-11 21:30 GMT-02:00 Arnout Vandecappelle <arnout@mind.be>:
>>>> On 11-11-15 23:18, Rodrigo Rebello wrote:
>> [snip]
>>>>> +diff --git a/configure b/configure
>>>>> +index cd219d8..a6f4101 100755
>>>>> +--- a/configure
>>>>> ++++ b/configure
>>>>> +@@ -1471,6 +1471,24 @@ for flag in $gcc_flags; do
>>>>> + done
>>>>> +
>>>>> + if test "$stack_protector" != "no"; then
>>>>> ++  cat > $TMPC << EOF
>>>>> ++void foo(const char *c);
>>>>
>>>>  This declaration is unnecessary.
>>>>
>>>
>>> Actually it is necessary, otherwise compiling the test code fails with:
>>>
>>> config-temp/qemu-conf.c:3:6: error: no previous prototype for ?foo?
>>> [-Werror=missing-prototypes]
>>>  void foo(const char *c)
>>>       ^
>>>
>>> Because in configure, line 410, -Wmissing-prototypes is added to
>>> QEMU_FLAGS (used in every compile test), and tests are run with
>>> -Werror by default, unless --disable-werror is passed to the configure
>>> script.
>>
>>  Right. And I guess declaring it static means that the whole function could be
>> elided so still no stack protection.
>>
>>  I wonder what would happen if LTO were enabled, since in that case the function
>> could be elided again...
>>
> 
> Indeed, I've run a test with LTO enabled and no stack protection code
> gets emitted due to the optimizations. Finding a test code fragment
> that works for all possibilities isn't so simple after all.
> 
> Maybe we should stick to the original v1 of this patch which disables
> SSP detection when it's known to be unsupported in the toolchain? Far
> from perfect, but at least it's more robust.

 It would be nicer to find a fundamental upstreamable solution, but yes, the
original solution was more robust.

> 
>>  To solve this fundamentally in buildroot, perhaps we should patch the gcc
>> wrapper to fail when -fstack-protector-* is passed when SSP is not enabled.
>>
> 
> Yes, that would be nice since it would serve as a solution to many
> other similar SSP detection failure cases. Perhaps I should look into
> that? Or wait until more people provide their views on this?

 Feel free to look into that. It shouldn't be too hard, we're already iterating
over all the arguments in the wrapper so you have an example of how to do it.
Just exit(1) immediately when you encounter an option that is not allowed.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2 1/1] qemu: add patch to fix SSP support detection

[Rodrigo Rebello]

Arnout, All,

2015-11-13 5:10 GMT-02:00 Arnout Vandecappelle <arnout@mind.be>:
[snip]
>>>  I wonder what would happen if LTO were enabled, since in that case the function
>>> could be elided again...
>>>
>>
>> Indeed, I've run a test with LTO enabled and no stack protection code
>> gets emitted due to the optimizations. Finding a test code fragment
>> that works for all possibilities isn't so simple after all.
>>
>> Maybe we should stick to the original v1 of this patch which disables
>> SSP detection when it's known to be unsupported in the toolchain? Far
>> from perfect, but at least it's more robust.
>
>  It would be nicer to find a fundamental upstreamable solution, but yes, the
> original solution was more robust.
>

Well, I've improved on the test code and the new version is not only
shorter (no need for foo()), but it also works with LTO enabled:

[snip]
  cat > $TMPC << EOF
int main(int argc, char *argv[])
{
    char arr[64], *p = arr, *c = argv[0];
    while (*c) {
        *p++ = *c++;
    }
    return 0;
}
EOF
[snip]

As this is more robust than the previous one, I've sent a new patch
upstream. It's already been reviewed by one person, but not yet
accepted:
https://patchwork.ozlabs.org/patch/543357/

I suspect I should wait for a while before submitting an update to the
buildroot list. However, given the great amount of patches the QEMU
project receives every day, I wonder if that will be looked@anytime
soon (if ever).

On the other hand, modifying the gcc wrapper to error out on
-fstack-protector-* when SSP is unsupported is straightforward and
I've already cooked up a patch for that. I could send it immediately
and maybe mark this qemu-specific patch as rejected or superseded.

Thoughts?

Regards,
Rodrigo

>>
>>>  To solve this fundamentally in buildroot, perhaps we should patch the gcc
>>> wrapper to fail when -fstack-protector-* is passed when SSP is not enabled.
>>>
>>
>> Yes, that would be nice since it would serve as a solution to many
>> other similar SSP detection failure cases. Perhaps I should look into
>> that? Or wait until more people provide their views on this?
>
>  Feel free to look into that. It shouldn't be too hard, we're already iterating
> over all the arguments in the wrapper so you have an example of how to do it.
> Just exit(1) immediately when you encounter an option that is not allowed.
>
>  Regards,
>  Arnout
>
> --
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2 1/1] qemu: add patch to fix SSP support detection

[Arnout Vandecappelle]

On 14-11-15 04:32, Rodrigo Rebello wrote:
> Arnout, All,
> 
> 2015-11-13 5:10 GMT-02:00 Arnout Vandecappelle <arnout@mind.be>:
> [snip]
>>>>  I wonder what would happen if LTO were enabled, since in that case the function
>>>> could be elided again...
>>>>
>>>
>>> Indeed, I've run a test with LTO enabled and no stack protection code
>>> gets emitted due to the optimizations. Finding a test code fragment
>>> that works for all possibilities isn't so simple after all.
>>>
>>> Maybe we should stick to the original v1 of this patch which disables
>>> SSP detection when it's known to be unsupported in the toolchain? Far
>>> from perfect, but at least it's more robust.
>>
>>  It would be nicer to find a fundamental upstreamable solution, but yes, the
>> original solution was more robust.
>>
> 
> Well, I've improved on the test code and the new version is not only
> shorter (no need for foo()), but it also works with LTO enabled:
> 
> [snip]
>   cat > $TMPC << EOF
> int main(int argc, char *argv[])
> {
>     char arr[64], *p = arr, *c = argv[0];
>     while (*c) {
>         *p++ = *c++;
>     }
>     return 0;
> }
> EOF

 Yep, this piece of code is perfect, since it _will_ smash the stack (if
strlen(argv[0]) > 64) so the SSP support _must_ be linked in. The only thing
that could happen in some distant future is that the compiler does sufficient
static analysis to detect the problem at compile time and errors out
immediately, but at the moment that's a bit far-fetched.

> [snip]
> 
> As this is more robust than the previous one, I've sent a new patch
> upstream. It's already been reviewed by one person, but not yet
> accepted:
> https://patchwork.ozlabs.org/patch/543357/
> 
> I suspect I should wait for a while before submitting an update to the
> buildroot list. However, given the great amount of patches the QEMU
> project receives every day, I wonder if that will be looked at anytime
> soon (if ever).

 No need to wait to submit it to buildroot IMHO.

> 
> On the other hand, modifying the gcc wrapper to error out on
> -fstack-protector-* when SSP is unsupported is straightforward and
> I've already cooked up a patch for that. I could send it immediately
> and maybe mark this qemu-specific patch as rejected or superseded.

 Yes, please do send that patch. It is however something that will probably only
be applied to next, while the qemu patch can be applied to master, so you should
probably send both of them.


 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2 1/1] qemu: add patch to fix SSP support detection

[Rodrigo Rebello]

Arnout,

2015-11-16 4:32 GMT-02:00 Arnout Vandecappelle <arnout@mind.be>:
[snip]
>
>  No need to wait to submit it to buildroot IMHO.
>
>>
>> On the other hand, modifying the gcc wrapper to error out on
>> -fstack-protector-* when SSP is unsupported is straightforward and
>> I've already cooked up a patch for that. I could send it immediately
>> and maybe mark this qemu-specific patch as rejected or superseded.
>
>  Yes, please do send that patch. It is however something that will probably only
> be applied to next, while the qemu patch can be applied to master, so you should
> probably send both of them.
>

OK then, I'll send them both and mark this one as superseded.

Regards,
Rodrigo

>
>  Regards,
>  Arnout
>
> --
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF