* [Qemu-devel] [PATCH 1/1] configure: use appropriate code fragment for -fstack-protector checks @ 2015-11-11 20:57 Rodrigo Rebello 2015-11-12 8:41 ` Markus Armbruster 0 siblings, 1 reply; 5+ messages in thread From: Rodrigo Rebello @ 2015-11-11 20:57 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-trivial, Rodrigo Rebello 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 <rprebello@gmail.com> --- 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); + return 0; +} +EOF gcc_flags="-fstack-protector-strong -fstack-protector-all" sp_on=0 for flag in $gcc_flags; do -- 2.1.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] configure: use appropriate code fragment for -fstack-protector checks 2015-11-11 20:57 [Qemu-devel] [PATCH 1/1] configure: use appropriate code fragment for -fstack-protector checks Rodrigo Rebello @ 2015-11-12 8:41 ` Markus Armbruster 2015-11-12 12:54 ` Rodrigo Rebello 0 siblings, 1 reply; 5+ messages in thread From: Markus Armbruster @ 2015-11-12 8:41 UTC (permalink / raw) To: Rodrigo Rebello; +Cc: qemu-trivial, qemu-devel Rodrigo Rebello <rprebello@gmail.com> 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 <rprebello@gmail.com> > --- > 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]); } > + return 0; > +} > +EOF > gcc_flags="-fstack-protector-strong -fstack-protector-all" > sp_on=0 > for flag in $gcc_flags; do ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] configure: use appropriate code fragment for -fstack-protector checks 2015-11-12 8:41 ` Markus Armbruster @ 2015-11-12 12:54 ` Rodrigo Rebello 2015-11-12 13:29 ` Markus Armbruster 0 siblings, 1 reply; 5+ messages in thread From: Rodrigo Rebello @ 2015-11-12 12:54 UTC (permalink / raw) To: Markus Armbruster; +Cc: qemu-trivial, qemu-devel Markus, 2015-11-12 6:41 GMT-02:00 Markus Armbruster <armbru@redhat.com>: > Rodrigo Rebello <rprebello@gmail.com> 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 <rprebello@gmail.com> >> --- >> 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? Regards, Rodrigo >> + return 0; >> +} >> +EOF >> gcc_flags="-fstack-protector-strong -fstack-protector-all" >> sp_on=0 >> for flag in $gcc_flags; do ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] configure: use appropriate code fragment for -fstack-protector checks 2015-11-12 12:54 ` Rodrigo Rebello @ 2015-11-12 13:29 ` Markus Armbruster 2015-11-12 13:54 ` Rodrigo Rebello 0 siblings, 1 reply; 5+ messages in thread From: Markus Armbruster @ 2015-11-12 13:29 UTC (permalink / raw) To: Rodrigo Rebello; +Cc: qemu-trivial, qemu-devel Rodrigo Rebello <rprebello@gmail.com> writes: > Markus, > > 2015-11-12 6:41 GMT-02:00 Markus Armbruster <armbru@redhat.com>: >> Rodrigo Rebello <rprebello@gmail.com> 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 <rprebello@gmail.com> >>> --- >>> 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] configure: use appropriate code fragment for -fstack-protector checks 2015-11-12 13:29 ` Markus Armbruster @ 2015-11-12 13:54 ` Rodrigo Rebello 0 siblings, 0 replies; 5+ messages in thread From: Rodrigo Rebello @ 2015-11-12 13:54 UTC (permalink / raw) To: Markus Armbruster; +Cc: qemu-trivial, qemu-devel Markus, 2015-11-12 11:29 GMT-02:00 Markus Armbruster <armbru@redhat.com>: > Rodrigo Rebello <rprebello@gmail.com> writes: > >> Markus, >> >> 2015-11-12 6:41 GMT-02:00 Markus Armbruster <armbru@redhat.com>: >>> Rodrigo Rebello <rprebello@gmail.com> 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 <rprebello@gmail.com> >>>> --- >>>> 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-11-12 13:54 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-11-11 20:57 [Qemu-devel] [PATCH 1/1] configure: use appropriate code fragment for -fstack-protector checks Rodrigo Rebello 2015-11-12 8:41 ` Markus Armbruster 2015-11-12 12:54 ` Rodrigo Rebello 2015-11-12 13:29 ` Markus Armbruster 2015-11-12 13:54 ` Rodrigo Rebello
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.