* [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags @ 2014-01-09 21:55 Steven Noonan 2014-01-09 21:55 ` [Qemu-devel] [PATCH 2/3] virtfs-proxy-helper.c: fix compile error Steven Noonan ` (3 more replies) 0 siblings, 4 replies; 16+ messages in thread From: Steven Noonan @ 2014-01-09 21:55 UTC (permalink / raw) To: qemu-devel; +Cc: Steven Noonan, Anthony Liguori From: Steven Noonan <snoonan@amazon.com> The -fstack-protector flag family is useful for ensuring safety and for debugging, but has a performance impact. Here's a boot time comparison between a QEMU build of qemu-system-arm with and without the -fstack-protector-all flag: # WITHOUT -fstack-protector-all [root@localhost ~]# systemd-analyze Startup finished in 1.744s (kernel) + 11.345s (initrd) + 47.164s (userspace) = 1min 255ms # WITH -fstack-protector-all [root@localhost ~]# systemd-analyze Startup finished in 1.843s (kernel) + 12.262s (initrd) + 1min 3.480s (userspace) = 1min 17.587s This is a sizable delta, and some users may wish to disable the flag. Signed-off-by: Steven Noonan <snoonan@amazon.com> Cc: Anthony Liguori <aliguori@amazon.com> --- configure | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/configure b/configure index 07b6be3..c091cdf 100755 --- a/configure +++ b/configure @@ -147,6 +147,7 @@ audio_win_int="" cc_i386=i386-pc-linux-gnu-gcc libs_qga="" debug_info="yes" +stack_protector="yes" # Don't accept a target_list environment variable. unset target_list @@ -879,6 +880,10 @@ for opt do ;; --disable-werror) werror="no" ;; + --enable-stack-protector) stack_protector="yes" + ;; + --disable-stack-protector) stack_protector="no" + ;; --disable-curses) curses="no" ;; --enable-curses) curses="yes" @@ -1117,6 +1122,7 @@ echo " --enable-sparse enable sparse checker" echo " --disable-sparse disable sparse checker (default)" echo " --disable-strip disable stripping binaries" echo " --disable-werror disable compilation abort on warning" +echo " --disable-stack-protector disable GCC-provided stack protection" echo " --disable-sdl disable SDL" echo " --enable-sdl enable SDL" echo " --disable-gtk disable gtk UI" @@ -1298,9 +1304,11 @@ for flag in $gcc_flags; do fi done -if compile_prog "-Werror -fstack-protector-all" "" ; then +if test "$stack_protector" = "yes" ; then + if compile_prog "-Werror -fstack-protector-all" "" ; then QEMU_CFLAGS="$QEMU_CFLAGS -fstack-protector-all" LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,-fstack-protector-all" + fi fi # Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC and -- 1.8.5.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 2/3] virtfs-proxy-helper.c: fix compile error 2014-01-09 21:55 [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags Steven Noonan @ 2014-01-09 21:55 ` Steven Noonan 2014-01-09 21:55 ` [Qemu-devel] [PATCH 3/3] Makefile: add bios-256k.bin to BLOBS Steven Noonan ` (2 subsequent siblings) 3 siblings, 0 replies; 16+ messages in thread From: Steven Noonan @ 2014-01-09 21:55 UTC (permalink / raw) To: qemu-devel; +Cc: Steven Noonan, Anthony Liguori From: Steven Noonan <snoonan@amazon.com> This is caused by a linux/xattr.h and sys/xattr.h incompatibility: In file included from /home/snoonan/Development/qemu/include/qemu/xattr.h:27:0, from fsdev/virtfs-proxy-helper.c:25: /usr/include/sys/xattr.h:31:3: error: expected identifier before numeric constant XATTR_CREATE = 1, /* set value, fail if attr already exists. */ ^ /home/snoonan/Development/qemu/qemu/rules.mak:25: recipe for target 'fsdev/virtfs-proxy-helper.o' failed Moving the include around resolves it. Signed-off-by: Steven Noonan <snoonan@amazon.com> Cc: Anthony Liguori <aliguori@amazon.com> --- fsdev/virtfs-proxy-helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c index 713a7b2..c10a085 100644 --- a/fsdev/virtfs-proxy-helper.c +++ b/fsdev/virtfs-proxy-helper.c @@ -12,7 +12,6 @@ #include <sys/resource.h> #include <getopt.h> #include <syslog.h> -#include <sys/capability.h> #include <sys/fsuid.h> #include <sys/vfs.h> #include <sys/ioctl.h> @@ -23,6 +22,7 @@ #include "qemu-common.h" #include "qemu/sockets.h" #include "qemu/xattr.h" +#include <sys/capability.h> #include "virtio-9p-marshal.h" #include "hw/9pfs/virtio-9p-proxy.h" #include "fsdev/virtio-9p-marshal.h" -- 1.8.5.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 3/3] Makefile: add bios-256k.bin to BLOBS 2014-01-09 21:55 [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags Steven Noonan 2014-01-09 21:55 ` [Qemu-devel] [PATCH 2/3] virtfs-proxy-helper.c: fix compile error Steven Noonan @ 2014-01-09 21:55 ` Steven Noonan 2014-01-09 23:27 ` Peter Maydell 2014-01-09 22:30 ` [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags Peter Maydell 2014-01-09 22:40 ` Paolo Bonzini 3 siblings, 1 reply; 16+ messages in thread From: Steven Noonan @ 2014-01-09 21:55 UTC (permalink / raw) To: qemu-devel; +Cc: Steven Noonan, Anthony Liguori From: Steven Noonan <snoonan@amazon.com> Signed-off-by: Steven Noonan <snoonan@amazon.com> Cc: Anthony Liguori <aliguori@amazon.com> --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index bdff4e4..807054b 100644 --- a/Makefile +++ b/Makefile @@ -290,7 +290,7 @@ common de-ch es fo fr-ca hu ja mk nl-be pt sl tr \ bepo cz ifdef INSTALL_BLOBS -BLOBS=bios.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \ +BLOBS=bios.bin bios-256k.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \ vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin \ acpi-dsdt.aml q35-acpi-dsdt.aml \ ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc QEMU,tcx.bin \ -- 1.8.5.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] Makefile: add bios-256k.bin to BLOBS 2014-01-09 21:55 ` [Qemu-devel] [PATCH 3/3] Makefile: add bios-256k.bin to BLOBS Steven Noonan @ 2014-01-09 23:27 ` Peter Maydell 2014-01-09 23:30 ` Noonan, Steven 0 siblings, 1 reply; 16+ messages in thread From: Peter Maydell @ 2014-01-09 23:27 UTC (permalink / raw) To: Steven Noonan; +Cc: Steven Noonan, QEMU Developers, Anthony Liguori On 9 January 2014 21:55, Steven Noonan <steven@uplinklabs.net> wrote: > From: Steven Noonan <snoonan@amazon.com> > > Signed-off-by: Steven Noonan <snoonan@amazon.com> > Cc: Anthony Liguori <aliguori@amazon.com> > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index bdff4e4..807054b 100644 > --- a/Makefile > +++ b/Makefile > @@ -290,7 +290,7 @@ common de-ch es fo fr-ca hu ja mk nl-be pt sl tr \ > bepo cz > > ifdef INSTALL_BLOBS > -BLOBS=bios.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \ > +BLOBS=bios.bin bios-256k.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \ We already have this patch on list and reviewed: http://patchwork.ozlabs.org/patch/299233/ Anthony should just apply that one :-) thanks -- PMM ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] Makefile: add bios-256k.bin to BLOBS 2014-01-09 23:27 ` Peter Maydell @ 2014-01-09 23:30 ` Noonan, Steven 2014-01-10 9:12 ` Markus Armbruster 0 siblings, 1 reply; 16+ messages in thread From: Noonan, Steven @ 2014-01-09 23:30 UTC (permalink / raw) To: Peter Maydell; +Cc: Steven Noonan, QEMU Developers, Anthony Liguori On Thu, Jan 09, 2014 at 11:27:47PM +0000, Peter Maydell wrote: > On 9 January 2014 21:55, Steven Noonan <steven@uplinklabs.net> wrote: > > From: Steven Noonan <snoonan@amazon.com> > > > > Signed-off-by: Steven Noonan <snoonan@amazon.com> > > Cc: Anthony Liguori <aliguori@amazon.com> > > --- > > Makefile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Makefile b/Makefile > > index bdff4e4..807054b 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -290,7 +290,7 @@ common de-ch es fo fr-ca hu ja mk nl-be pt sl tr \ > > bepo cz > > > > ifdef INSTALL_BLOBS > > -BLOBS=bios.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \ > > +BLOBS=bios.bin bios-256k.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \ > > We already have this patch on list and reviewed: > http://patchwork.ozlabs.org/patch/299233/ > > Anthony should just apply that one :-) Agreed. This is what happens when I don't search the list before writing a patch. :) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] Makefile: add bios-256k.bin to BLOBS 2014-01-09 23:30 ` Noonan, Steven @ 2014-01-10 9:12 ` Markus Armbruster 0 siblings, 0 replies; 16+ messages in thread From: Markus Armbruster @ 2014-01-10 9:12 UTC (permalink / raw) To: Noonan, Steven Cc: Peter Maydell, Steven Noonan, QEMU Developers, Anthony Liguori "Noonan, Steven" <snoonan@amazon.com> writes: > On Thu, Jan 09, 2014 at 11:27:47PM +0000, Peter Maydell wrote: >> On 9 January 2014 21:55, Steven Noonan <steven@uplinklabs.net> wrote: >> > From: Steven Noonan <snoonan@amazon.com> >> > >> > Signed-off-by: Steven Noonan <snoonan@amazon.com> >> > Cc: Anthony Liguori <aliguori@amazon.com> >> > --- >> > Makefile | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/Makefile b/Makefile >> > index bdff4e4..807054b 100644 >> > --- a/Makefile >> > +++ b/Makefile >> > @@ -290,7 +290,7 @@ common de-ch es fo fr-ca hu ja mk nl-be pt sl >> > tr \ >> > bepo cz >> > >> > ifdef INSTALL_BLOBS >> > -BLOBS=bios.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \ >> > +BLOBS=bios.bin bios-256k.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \ >> >> We already have this patch on list and reviewed: >> http://patchwork.ozlabs.org/patch/299233/ >> >> Anthony should just apply that one :-) > > Agreed. This is what happens when I don't search the list before writing > a patch. :) Actually, it's what happens when patches don't get applied in a timely manner. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags 2014-01-09 21:55 [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags Steven Noonan 2014-01-09 21:55 ` [Qemu-devel] [PATCH 2/3] virtfs-proxy-helper.c: fix compile error Steven Noonan 2014-01-09 21:55 ` [Qemu-devel] [PATCH 3/3] Makefile: add bios-256k.bin to BLOBS Steven Noonan @ 2014-01-09 22:30 ` Peter Maydell 2014-01-09 22:40 ` Paolo Bonzini 3 siblings, 0 replies; 16+ messages in thread From: Peter Maydell @ 2014-01-09 22:30 UTC (permalink / raw) To: Steven Noonan; +Cc: Steven Noonan, QEMU Developers, Anthony Liguori On 9 January 2014 21:55, Steven Noonan <steven@uplinklabs.net> wrote: > From: Steven Noonan <snoonan@amazon.com> > > The -fstack-protector flag family is useful for ensuring safety and for > debugging, but has a performance impact. Here's a boot time comparison between > a QEMU build of qemu-system-arm with and without the -fstack-protector-all > flag: > > # WITHOUT -fstack-protector-all > [root@localhost ~]# systemd-analyze > Startup finished in 1.744s (kernel) + 11.345s (initrd) + 47.164s (userspace) = 1min 255ms > > # WITH -fstack-protector-all > [root@localhost ~]# systemd-analyze > Startup finished in 1.843s (kernel) + 12.262s (initrd) + 1min 3.480s (userspace) = 1min 17.587s > > This is a sizable delta, and some users may wish to disable the flag. How about benchmarking the intermediate level of protection, ie just "-fstack-protector"? Maybe that's a good enough compromise between security and speed that we don't need to mess with configure... (IIRC there have been discussions before about why we have the -all variant specifically but I don't recall anybody coming up with a convincing argument.) thanks -- PMM ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags 2014-01-09 21:55 [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags Steven Noonan ` (2 preceding siblings ...) 2014-01-09 22:30 ` [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags Peter Maydell @ 2014-01-09 22:40 ` Paolo Bonzini 2014-01-09 23:18 ` Brad Smith 3 siblings, 1 reply; 16+ messages in thread From: Paolo Bonzini @ 2014-01-09 22:40 UTC (permalink / raw) To: Steven Noonan; +Cc: Steven Noonan, qemu-devel, Anthony Liguori Il 09/01/2014 22:55, Steven Noonan ha scritto: > From: Steven Noonan <snoonan@amazon.com> > > The -fstack-protector flag family is useful for ensuring safety and for > debugging, but has a performance impact. Here's a boot time comparison between > a QEMU build of qemu-system-arm with and without the -fstack-protector-all > flag: > > # WITHOUT -fstack-protector-all > [root@localhost ~]# systemd-analyze > Startup finished in 1.744s (kernel) + 11.345s (initrd) + 47.164s (userspace) = 1min 255ms > > # WITH -fstack-protector-all > [root@localhost ~]# systemd-analyze > Startup finished in 1.843s (kernel) + 12.262s (initrd) + 1min 3.480s (userspace) = 1min 17.587s Can you try -fstack-protector-strong? Probably the right thing to do is to pick in order -fstack-protector-strong, -fstack-protector, and nothing. Paolo ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags 2014-01-09 22:40 ` Paolo Bonzini @ 2014-01-09 23:18 ` Brad Smith 2014-01-09 23:31 ` Noonan, Steven 2014-01-11 2:36 ` Noonan, Steven 0 siblings, 2 replies; 16+ messages in thread From: Brad Smith @ 2014-01-09 23:18 UTC (permalink / raw) To: Paolo Bonzini, Steven Noonan; +Cc: Steven Noonan, qemu-devel, Anthony Liguori On 09/01/14 5:40 PM, Paolo Bonzini wrote: > Il 09/01/2014 22:55, Steven Noonan ha scritto: >> From: Steven Noonan <snoonan@amazon.com> >> >> The -fstack-protector flag family is useful for ensuring safety and for >> debugging, but has a performance impact. Here's a boot time comparison between >> a QEMU build of qemu-system-arm with and without the -fstack-protector-all >> flag: >> >> # WITHOUT -fstack-protector-all >> [root@localhost ~]# systemd-analyze >> Startup finished in 1.744s (kernel) + 11.345s (initrd) + 47.164s (userspace) = 1min 255ms >> >> # WITH -fstack-protector-all >> [root@localhost ~]# systemd-analyze >> Startup finished in 1.843s (kernel) + 12.262s (initrd) + 1min 3.480s (userspace) = 1min 17.587s > > Can you try -fstack-protector-strong? > > Probably the right thing to do is to pick in order > -fstack-protector-strong, -fstack-protector, and nothing. +1 -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags 2014-01-09 23:18 ` Brad Smith @ 2014-01-09 23:31 ` Noonan, Steven 2014-01-11 2:36 ` Noonan, Steven 1 sibling, 0 replies; 16+ messages in thread From: Noonan, Steven @ 2014-01-09 23:31 UTC (permalink / raw) To: Brad Smith; +Cc: Paolo Bonzini, Steven Noonan, qemu-devel, Anthony Liguori On Thu, Jan 09, 2014 at 06:18:07PM -0500, Brad Smith wrote: > On 09/01/14 5:40 PM, Paolo Bonzini wrote: > >Il 09/01/2014 22:55, Steven Noonan ha scritto: > >>From: Steven Noonan <snoonan@amazon.com> > >> > >>The -fstack-protector flag family is useful for ensuring safety and for > >>debugging, but has a performance impact. Here's a boot time comparison between > >>a QEMU build of qemu-system-arm with and without the -fstack-protector-all > >>flag: > >> > >> # WITHOUT -fstack-protector-all > >> [root@localhost ~]# systemd-analyze > >> Startup finished in 1.744s (kernel) + 11.345s (initrd) + 47.164s (userspace) = 1min 255ms > >> > >> # WITH -fstack-protector-all > >> [root@localhost ~]# systemd-analyze > >> Startup finished in 1.843s (kernel) + 12.262s (initrd) + 1min 3.480s (userspace) = 1min 17.587s > > > >Can you try -fstack-protector-strong? > > > >Probably the right thing to do is to pick in order > >-fstack-protector-strong, -fstack-protector, and nothing. > > +1 > I think there should still be an option to turn it off, but I agree that there are probably better flags than -fstack-protector-all. I'll try those out and post an update here probably later this evening. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags 2014-01-09 23:18 ` Brad Smith 2014-01-09 23:31 ` Noonan, Steven @ 2014-01-11 2:36 ` Noonan, Steven 2014-01-11 7:46 ` Stefan Weil 1 sibling, 1 reply; 16+ messages in thread From: Noonan, Steven @ 2014-01-11 2:36 UTC (permalink / raw) To: Brad Smith; +Cc: Paolo Bonzini, Steven Noonan, qemu-devel, Anthony Liguori [-- Attachment #1: Type: text/plain, Size: 3019 bytes --] On Thu, Jan 09, 2014 at 06:18:07PM -0500, Brad Smith wrote: > On 09/01/14 5:40 PM, Paolo Bonzini wrote: > >Il 09/01/2014 22:55, Steven Noonan ha scritto: > >>From: Steven Noonan <snoonan@amazon.com> > >> > >>The -fstack-protector flag family is useful for ensuring safety and for > >>debugging, but has a performance impact. Here's a boot time comparison between > >>a QEMU build of qemu-system-arm with and without the -fstack-protector-all > >>flag: > >> > >> # WITHOUT -fstack-protector-all > >> [root@localhost ~]# systemd-analyze > >> Startup finished in 1.744s (kernel) + 11.345s (initrd) + 47.164s (userspace) = 1min 255ms > >> > >> # WITH -fstack-protector-all > >> [root@localhost ~]# systemd-analyze > >> Startup finished in 1.843s (kernel) + 12.262s (initrd) + 1min 3.480s (userspace) = 1min 17.587s > > > >Can you try -fstack-protector-strong? > > > >Probably the right thing to do is to pick in order > >-fstack-protector-strong, -fstack-protector, and nothing. > > +1 > OK, in order to get -fstack-protector-strong I had to get a Fedora box up and running, so the compiler and build/execution environment are different. I took three samples from each build, applying a clean QCOW2 snapshot before each run to avoid cross-boot confounding variables: # -fstack-protector-all Startup finished in 1.810s (kernel) + 12.331s (initrd) + 49.016s (userspace) = 1min 3.159s Startup finished in 1.801s (kernel) + 12.287s (initrd) + 47.925s (userspace) = 1min 2.013s Startup finished in 1.812s (kernel) + 12.302s (initrd) + 47.995s (userspace) = 1min 2.111s # -fstack-protector-strong Startup finished in 1.744s (kernel) + 11.223s (initrd) + 44.688s (userspace) = 57.657s Startup finished in 1.721s (kernel) + 11.222s (initrd) + 44.194s (userspace) = 57.138s Startup finished in 1.693s (kernel) + 11.250s (initrd) + 44.426s (userspace) = 57.370s # -fstack-protector Startup finished in 1.705s (kernel) + 11.409s (initrd) + 43.563s (userspace) = 56.677s Startup finished in 1.877s (kernel) + 11.137s (initrd) + 43.719s (userspace) = 56.734s Startup finished in 1.708s (kernel) + 11.141s (initrd) + 43.628s (userspace) = 56.478s # no stack protector Startup finished in 1.743s (kernel) + 11.190s (initrd) + 43.709s (userspace) = 56.643s Startup finished in 1.763s (kernel) + 11.216s (initrd) + 43.767s (userspace) = 56.747s Startup finished in 1.711s (kernel) + 11.283s (initrd) + 43.878s (userspace) = 56.873s There are a few points to note: - stack-protector-all doesn't cost nearly as much in this build, for reasons unclear to me. It looks like the kernel/initrd timings are rather closely reproduced, but the userspace timing varies quite a bit. - stack-protector-strong's performance hit is indeed much less than stack-protector-all. - The builds with the plain old -fstack-protector flag and without any stack protector are tied for the lead in terms of performance. Amended patch attached. [-- Attachment #2: 0001-configure-add-option-to-disable-fstack-protector-fla.patch --] [-- Type: text/x-diff, Size: 3726 bytes --] >From cad2b5990debede2bc8dc8552904b2ef9e14af22 Mon Sep 17 00:00:00 2001 From: Steven Noonan <snoonan@amazon.com> Date: Mon, 6 Jan 2014 13:39:20 -0800 Subject: [PATCH] configure: add option to disable -fstack-protector flags The -fstack-protector flag family is useful for ensuring safety and for debugging, but has a performance impact. Here are some boot time comparisons of the various versions of -fstack-protector using qemu-system-arm on an x86_64 host: # -fstack-protector-all Startup finished in 1.810s (kernel) + 12.331s (initrd) + 49.016s (userspace) = 1min 3.159s Startup finished in 1.801s (kernel) + 12.287s (initrd) + 47.925s (userspace) = 1min 2.013s Startup finished in 1.812s (kernel) + 12.302s (initrd) + 47.995s (userspace) = 1min 2.111s # -fstack-protector-strong Startup finished in 1.744s (kernel) + 11.223s (initrd) + 44.688s (userspace) = 57.657s Startup finished in 1.721s (kernel) + 11.222s (initrd) + 44.194s (userspace) = 57.138s Startup finished in 1.693s (kernel) + 11.250s (initrd) + 44.426s (userspace) = 57.370s # -fstack-protector Startup finished in 1.705s (kernel) + 11.409s (initrd) + 43.563s (userspace) = 56.677s Startup finished in 1.877s (kernel) + 11.137s (initrd) + 43.719s (userspace) = 56.734s Startup finished in 1.708s (kernel) + 11.141s (initrd) + 43.628s (userspace) = 56.478s # no stack protector Startup finished in 1.743s (kernel) + 11.190s (initrd) + 43.709s (userspace) = 56.643s Startup finished in 1.763s (kernel) + 11.216s (initrd) + 43.767s (userspace) = 56.747s Startup finished in 1.711s (kernel) + 11.283s (initrd) + 43.878s (userspace) = 56.873s This patch introduces a configure option to disable the stack protector entirely, and conditional stack protector flag selection (in order, based on availability): -fstack-protector-strong, -fstack-protector, no stack protector. Signed-off-by: Steven Noonan <snoonan@amazon.com> --- configure | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/configure b/configure index 07b6be3..a485e94 100755 --- a/configure +++ b/configure @@ -147,6 +147,7 @@ audio_win_int="" cc_i386=i386-pc-linux-gnu-gcc libs_qga="" debug_info="yes" +stack_protector="yes" # Don't accept a target_list environment variable. unset target_list @@ -879,6 +880,10 @@ for opt do ;; --disable-werror) werror="no" ;; + --enable-stack-protector) stack_protector="yes" + ;; + --disable-stack-protector) stack_protector="no" + ;; --disable-curses) curses="no" ;; --enable-curses) curses="yes" @@ -1117,6 +1122,7 @@ echo " --enable-sparse enable sparse checker" echo " --disable-sparse disable sparse checker (default)" echo " --disable-strip disable stripping binaries" echo " --disable-werror disable compilation abort on warning" +echo " --disable-stack-protector disable GCC-provided stack protection" echo " --disable-sdl disable SDL" echo " --enable-sdl enable SDL" echo " --disable-gtk disable gtk UI" @@ -1298,9 +1304,15 @@ for flag in $gcc_flags; do fi done -if compile_prog "-Werror -fstack-protector-all" "" ; then - QEMU_CFLAGS="$QEMU_CFLAGS -fstack-protector-all" - LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,-fstack-protector-all" +if test "$stack_protector" = "yes" ; then + gcc_flags="-fstack-protector-strong -fstack-protector" + for flag in $gcc_flags; do + if compile_prog "-Werror $flag" "" ; then + QEMU_CFLAGS="$QEMU_CFLAGS $flag" + LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,$flag" + break + fi + done fi # Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC and -- 1.8.5.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags 2014-01-11 2:36 ` Noonan, Steven @ 2014-01-11 7:46 ` Stefan Weil 2014-01-13 11:53 ` Paolo Bonzini 0 siblings, 1 reply; 16+ messages in thread From: Stefan Weil @ 2014-01-11 7:46 UTC (permalink / raw) To: Noonan, Steven, Brad Smith Cc: Paolo Bonzini, Steven Noonan, qemu-devel, Anthony Liguori Hi Steven, --disable-stack-protector would also be useful for platforms which make debugging of executables with stack protection difficult. When I must debug Windows executables, I always disable stack protection, because otherwise the stack back traces are unreadable. So, for MinGW it might be reasonable to set the default to no stack protection if --enable-debug is selected. This requires some modifications in your patch. # Don't set it to "yes" initially: stack_protector="" # Do the compile test if it is not "no": if test "$stack_protector" != "no"; then The usual logic is do nothing if the user says "no". Run the compile tests otherwise. Show an error message if the compile tests fail and the user said "yes". See the code which handles $pie for an example. Please send your next patch inline - this makes it easier to add comments. Best regards Stefan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags 2014-01-11 7:46 ` Stefan Weil @ 2014-01-13 11:53 ` Paolo Bonzini 2014-01-13 20:00 ` [Qemu-devel] [PATCH] " Steven Noonan 0 siblings, 1 reply; 16+ messages in thread From: Paolo Bonzini @ 2014-01-13 11:53 UTC (permalink / raw) To: Stefan Weil Cc: Steven Noonan, Noonan, Steven, qemu-devel, Anthony Liguori, Brad Smith Il 11/01/2014 08:46, Stefan Weil ha scritto: > Hi Steven, > > --disable-stack-protector would also be useful for platforms which make > debugging of executables with stack protection difficult. When I must > debug Windows executables, I always disable stack protection, because > otherwise the stack back traces are unreadable. > > So, for MinGW it might be reasonable to set the default to no stack > protection if --enable-debug is selected. This requires some > modifications in your patch. > > # Don't set it to "yes" initially: > stack_protector="" > > # Do the compile test if it is not "no": > if test "$stack_protector" != "no"; then Apart from this little detail, the patch looks good. Steven, please resend the patch as a top-level message with the patch inline rather than attached. This is needed so that Anthony's script will pick it up. Including these changes would be nice too. Paolo > The usual logic is do nothing if the user says "no". Run the compile > tests otherwise. Show an error message if the compile tests fail and the > user said "yes". See the code which handles $pie for an example. > > Please send your next patch inline - this makes it easier to add comments. > > Best regards > > Stefan > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH] configure: add option to disable -fstack-protector flags 2014-01-13 11:53 ` Paolo Bonzini @ 2014-01-13 20:00 ` Steven Noonan 2014-01-13 20:27 ` Stefan Weil 0 siblings, 1 reply; 16+ messages in thread From: Steven Noonan @ 2014-01-13 20:00 UTC (permalink / raw) To: qemu-devel; +Cc: Steven Noonan, Anthony Liguori The -fstack-protector flag family is useful for ensuring safety and for debugging, but has a performance impact. Here are some boot time comparisons of the various versions of -fstack-protector using qemu-system-arm on an x86_64 host: # -fstack-protector-all Startup finished in 1.810s (kernel) + 12.331s (initrd) + 49.016s (userspace) = 1min 3.159s Startup finished in 1.801s (kernel) + 12.287s (initrd) + 47.925s (userspace) = 1min 2.013s Startup finished in 1.812s (kernel) + 12.302s (initrd) + 47.995s (userspace) = 1min 2.111s # -fstack-protector-strong Startup finished in 1.744s (kernel) + 11.223s (initrd) + 44.688s (userspace) = 57.657s Startup finished in 1.721s (kernel) + 11.222s (initrd) + 44.194s (userspace) = 57.138s Startup finished in 1.693s (kernel) + 11.250s (initrd) + 44.426s (userspace) = 57.370s # -fstack-protector Startup finished in 1.705s (kernel) + 11.409s (initrd) + 43.563s (userspace) = 56.677s Startup finished in 1.877s (kernel) + 11.137s (initrd) + 43.719s (userspace) = 56.734s Startup finished in 1.708s (kernel) + 11.141s (initrd) + 43.628s (userspace) = 56.478s # no stack protector Startup finished in 1.743s (kernel) + 11.190s (initrd) + 43.709s (userspace) = 56.643s Startup finished in 1.763s (kernel) + 11.216s (initrd) + 43.767s (userspace) = 56.747s Startup finished in 1.711s (kernel) + 11.283s (initrd) + 43.878s (userspace) = 56.873s This patch introduces a configure option to disable the stack protector entirely, and conditional stack protector flag selection (in order, based on availability): -fstack-protector-strong, -fstack-protector, no stack protector. Signed-off-by: Steven Noonan <snoonan@amazon.com> --- configure | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/configure b/configure index 07b6be3..c9c0b2c 100755 --- a/configure +++ b/configure @@ -147,6 +147,7 @@ audio_win_int="" cc_i386=i386-pc-linux-gnu-gcc libs_qga="" debug_info="yes" +stack_protector="" # Don't accept a target_list environment variable. unset target_list @@ -879,6 +880,10 @@ for opt do ;; --disable-werror) werror="no" ;; + --enable-stack-protector) stack_protector="yes" + ;; + --disable-stack-protector) stack_protector="no" + ;; --disable-curses) curses="no" ;; --enable-curses) curses="yes" @@ -1117,6 +1122,7 @@ echo " --enable-sparse enable sparse checker" echo " --disable-sparse disable sparse checker (default)" echo " --disable-strip disable stripping binaries" echo " --disable-werror disable compilation abort on warning" +echo " --disable-stack-protector disable GCC-provided stack protection" echo " --disable-sdl disable SDL" echo " --enable-sdl enable SDL" echo " --disable-gtk disable gtk UI" @@ -1298,9 +1304,15 @@ for flag in $gcc_flags; do fi done -if compile_prog "-Werror -fstack-protector-all" "" ; then - QEMU_CFLAGS="$QEMU_CFLAGS -fstack-protector-all" - LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,-fstack-protector-all" +if test "$stack_protector" != "no" ; then + gcc_flags="-fstack-protector-strong -fstack-protector" + for flag in $gcc_flags; do + if compile_prog "-Werror $flag" "" ; then + QEMU_CFLAGS="$QEMU_CFLAGS $flag" + LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,$flag" + break + fi + done fi # Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC and -- 1.8.5.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add option to disable -fstack-protector flags 2014-01-13 20:00 ` [Qemu-devel] [PATCH] " Steven Noonan @ 2014-01-13 20:27 ` Stefan Weil 2014-01-13 20:38 ` Noonan, Steven 0 siblings, 1 reply; 16+ messages in thread From: Stefan Weil @ 2014-01-13 20:27 UTC (permalink / raw) To: Steven Noonan, qemu-devel; +Cc: Paolo Bonzini, Steven Noonan, Anthony Liguori Hi, please see comments below. Am 13.01.2014 21:00, schrieb Steven Noonan: > The -fstack-protector flag family is useful for ensuring safety and for > debugging, but has a performance impact. Here are some boot time comparisons of > the various versions of -fstack-protector using qemu-system-arm on an x86_64 > host: > > # -fstack-protector-all > Startup finished in 1.810s (kernel) + 12.331s (initrd) + 49.016s (userspace) = 1min 3.159s > Startup finished in 1.801s (kernel) + 12.287s (initrd) + 47.925s (userspace) = 1min 2.013s > Startup finished in 1.812s (kernel) + 12.302s (initrd) + 47.995s (userspace) = 1min 2.111s > > # -fstack-protector-strong > Startup finished in 1.744s (kernel) + 11.223s (initrd) + 44.688s (userspace) = 57.657s > Startup finished in 1.721s (kernel) + 11.222s (initrd) + 44.194s (userspace) = 57.138s > Startup finished in 1.693s (kernel) + 11.250s (initrd) + 44.426s (userspace) = 57.370s > > # -fstack-protector > Startup finished in 1.705s (kernel) + 11.409s (initrd) + 43.563s (userspace) = 56.677s > Startup finished in 1.877s (kernel) + 11.137s (initrd) + 43.719s (userspace) = 56.734s > Startup finished in 1.708s (kernel) + 11.141s (initrd) + 43.628s (userspace) = 56.478s > > # no stack protector > Startup finished in 1.743s (kernel) + 11.190s (initrd) + 43.709s (userspace) = 56.643s > Startup finished in 1.763s (kernel) + 11.216s (initrd) + 43.767s (userspace) = 56.747s > Startup finished in 1.711s (kernel) + 11.283s (initrd) + 43.878s (userspace) = 56.873s > > This patch introduces a configure option to disable the stack protector > entirely, and conditional stack protector flag selection (in order, based on > availability): -fstack-protector-strong, -fstack-protector, no stack protector. > > Signed-off-by: Steven Noonan <snoonan@amazon.com> > --- > configure | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/configure b/configure > index 07b6be3..c9c0b2c 100755 > --- a/configure > +++ b/configure > @@ -147,6 +147,7 @@ audio_win_int="" > cc_i386=i386-pc-linux-gnu-gcc > libs_qga="" > debug_info="yes" > +stack_protector="" > > # Don't accept a target_list environment variable. > unset target_list > @@ -879,6 +880,10 @@ for opt do > ;; > --disable-werror) werror="no" > ;; > + --enable-stack-protector) stack_protector="yes" > + ;; > + --disable-stack-protector) stack_protector="no" > + ;; > --disable-curses) curses="no" > ;; > --enable-curses) curses="yes" > @@ -1117,6 +1122,7 @@ echo " --enable-sparse enable sparse checker" > echo " --disable-sparse disable sparse checker (default)" > echo " --disable-strip disable stripping binaries" > echo " --disable-werror disable compilation abort on warning" > +echo " --disable-stack-protector disable GCC-provided stack protection" Clang also supports stack protection AFAIK, so "GCC-provided" can be removed here (or replaced by "compiler"). > echo " --disable-sdl disable SDL" > echo " --enable-sdl enable SDL" > echo " --disable-gtk disable gtk UI" > @@ -1298,9 +1304,15 @@ for flag in $gcc_flags; do > fi > done > > -if compile_prog "-Werror -fstack-protector-all" "" ; then > - QEMU_CFLAGS="$QEMU_CFLAGS -fstack-protector-all" > - LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,-fstack-protector-all" > +if test "$stack_protector" != "no" ; then > + gcc_flags="-fstack-protector-strong -fstack-protector" > + for flag in $gcc_flags; do > + if compile_prog "-Werror $flag" "" ; then > + QEMU_CFLAGS="$QEMU_CFLAGS $flag" > + LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,$flag" > + break > + fi > + done > fi > > # Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC and > Reviewed-by: Stefan Weil <sw@weilnetz.de> I think this patch can be used as a base for further improvements (MinGW specific settings, error handling when user's choice does not work). Maybe you will have to resend the patch as a top level patch (don't use the reply function of your mailer). As Paolo said, Anthony might overwise be unable to pick it up with his scripts. Regards Stefan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add option to disable -fstack-protector flags 2014-01-13 20:27 ` Stefan Weil @ 2014-01-13 20:38 ` Noonan, Steven 0 siblings, 0 replies; 16+ messages in thread From: Noonan, Steven @ 2014-01-13 20:38 UTC (permalink / raw) To: Stefan Weil; +Cc: Paolo Bonzini, Steven Noonan, qemu-devel, Anthony Liguori On Mon, Jan 13, 2014 at 09:27:42PM +0100, Stefan Weil wrote: > Hi, > > please see comments below. > > Am 13.01.2014 21:00, schrieb Steven Noonan: > > The -fstack-protector flag family is useful for ensuring safety and for > > debugging, but has a performance impact. Here are some boot time comparisons of > > the various versions of -fstack-protector using qemu-system-arm on an x86_64 > > host: > > > > # -fstack-protector-all > > Startup finished in 1.810s (kernel) + 12.331s (initrd) + 49.016s (userspace) = 1min 3.159s > > Startup finished in 1.801s (kernel) + 12.287s (initrd) + 47.925s (userspace) = 1min 2.013s > > Startup finished in 1.812s (kernel) + 12.302s (initrd) + 47.995s (userspace) = 1min 2.111s > > > > # -fstack-protector-strong > > Startup finished in 1.744s (kernel) + 11.223s (initrd) + 44.688s (userspace) = 57.657s > > Startup finished in 1.721s (kernel) + 11.222s (initrd) + 44.194s (userspace) = 57.138s > > Startup finished in 1.693s (kernel) + 11.250s (initrd) + 44.426s (userspace) = 57.370s > > > > # -fstack-protector > > Startup finished in 1.705s (kernel) + 11.409s (initrd) + 43.563s (userspace) = 56.677s > > Startup finished in 1.877s (kernel) + 11.137s (initrd) + 43.719s (userspace) = 56.734s > > Startup finished in 1.708s (kernel) + 11.141s (initrd) + 43.628s (userspace) = 56.478s > > > > # no stack protector > > Startup finished in 1.743s (kernel) + 11.190s (initrd) + 43.709s (userspace) = 56.643s > > Startup finished in 1.763s (kernel) + 11.216s (initrd) + 43.767s (userspace) = 56.747s > > Startup finished in 1.711s (kernel) + 11.283s (initrd) + 43.878s (userspace) = 56.873s > > > > This patch introduces a configure option to disable the stack protector > > entirely, and conditional stack protector flag selection (in order, based on > > availability): -fstack-protector-strong, -fstack-protector, no stack protector. > > > > Signed-off-by: Steven Noonan <snoonan@amazon.com> > > --- > > configure | 18 +++++++++++++++--- > > 1 file changed, 15 insertions(+), 3 deletions(-) > > > > diff --git a/configure b/configure > > index 07b6be3..c9c0b2c 100755 > > --- a/configure > > +++ b/configure > > @@ -147,6 +147,7 @@ audio_win_int="" > > cc_i386=i386-pc-linux-gnu-gcc > > libs_qga="" > > debug_info="yes" > > +stack_protector="" > > > > # Don't accept a target_list environment variable. > > unset target_list > > @@ -879,6 +880,10 @@ for opt do > > ;; > > --disable-werror) werror="no" > > ;; > > + --enable-stack-protector) stack_protector="yes" > > + ;; > > + --disable-stack-protector) stack_protector="no" > > + ;; > > --disable-curses) curses="no" > > ;; > > --enable-curses) curses="yes" > > @@ -1117,6 +1122,7 @@ echo " --enable-sparse enable sparse checker" > > echo " --disable-sparse disable sparse checker (default)" > > echo " --disable-strip disable stripping binaries" > > echo " --disable-werror disable compilation abort on warning" > > +echo " --disable-stack-protector disable GCC-provided stack protection" > > > Clang also supports stack protection AFAIK, so "GCC-provided" can be > removed here (or replaced by "compiler"). > Fixed in v3 of the patch. > > > echo " --disable-sdl disable SDL" > > echo " --enable-sdl enable SDL" > > echo " --disable-gtk disable gtk UI" > > @@ -1298,9 +1304,15 @@ for flag in $gcc_flags; do > > fi > > done > > > > -if compile_prog "-Werror -fstack-protector-all" "" ; then > > - QEMU_CFLAGS="$QEMU_CFLAGS -fstack-protector-all" > > - LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,-fstack-protector-all" > > +if test "$stack_protector" != "no" ; then > > + gcc_flags="-fstack-protector-strong -fstack-protector" > > + for flag in $gcc_flags; do > > + if compile_prog "-Werror $flag" "" ; then > > + QEMU_CFLAGS="$QEMU_CFLAGS $flag" > > + LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,$flag" > > + break > > + fi > > + done > > fi > > > > # Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC and > > > > > Reviewed-by: Stefan Weil <sw@weilnetz.de> > > I think this patch can be used as a base for further improvements (MinGW > specific settings, error handling when user's choice does not work). > > Maybe you will have to resend the patch as a top level patch (don't use > the reply function of your mailer). As Paolo said, Anthony might > overwise be unable to pick it up with his scripts. Thanks, I've done that now. - Steven ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-01-13 20:38 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-01-09 21:55 [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags Steven Noonan 2014-01-09 21:55 ` [Qemu-devel] [PATCH 2/3] virtfs-proxy-helper.c: fix compile error Steven Noonan 2014-01-09 21:55 ` [Qemu-devel] [PATCH 3/3] Makefile: add bios-256k.bin to BLOBS Steven Noonan 2014-01-09 23:27 ` Peter Maydell 2014-01-09 23:30 ` Noonan, Steven 2014-01-10 9:12 ` Markus Armbruster 2014-01-09 22:30 ` [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags Peter Maydell 2014-01-09 22:40 ` Paolo Bonzini 2014-01-09 23:18 ` Brad Smith 2014-01-09 23:31 ` Noonan, Steven 2014-01-11 2:36 ` Noonan, Steven 2014-01-11 7:46 ` Stefan Weil 2014-01-13 11:53 ` Paolo Bonzini 2014-01-13 20:00 ` [Qemu-devel] [PATCH] " Steven Noonan 2014-01-13 20:27 ` Stefan Weil 2014-01-13 20:38 ` Noonan, Steven
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).