[libvirt] [PATCH] Remove ssp buffer size setting
This option only makes sense with -fstack-protector. With -fstack-protector-all, even functions with buffers smaller than this are protected. https://bugzilla.redhat.com/show_bug.cgi?id=1105456 --- m4/virt-compile-warnings.m4 | 8 -------- 1 file changed, 8 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 574fbc4..ebc931d 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -171,14 +171,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl be great overhead in adding -fstack-protector-all instead dnl wantwarn="$wantwarn -fstack-protector" wantwarn="$wantwarn -fstack-protector-all" - wantwarn="$wantwarn --param=ssp-buffer-size=4" - dnl Even though it supports it, clang complains about - dnl use of --param=ssp-buffer-size=4 unless used with - dnl the -c arg. It doesn't like it when used with args - dnl that just link together .o files. Unfortunately - dnl we can't avoid that with automake, so we must turn - dnl off the following clang specific warning - wantwarn="$wantwarn -Wno-unused-command-line-argument" ;; *-*-freebsd*) dnl FreeBSD ships old gcc 4.2.1 which doesn't handle -- 1.8.3.2
On Fri, Jun 06, 2014 at 11:40:24AM +0200, Ján Tomko wrote:
This option only makes sense with -fstack-protector. With -fstack-protector-all, even functions with buffers smaller than this are protected.
https://bugzilla.redhat.com/show_bug.cgi?id=1105456 --- m4/virt-compile-warnings.m4 | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 574fbc4..ebc931d 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -171,14 +171,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl be great overhead in adding -fstack-protector-all instead dnl wantwarn="$wantwarn -fstack-protector" wantwarn="$wantwarn -fstack-protector-all" - wantwarn="$wantwarn --param=ssp-buffer-size=4"
It would be nice to keep that line in here with the explanation that -fstack-protector-all does not make use of that param.
- dnl Even though it supports it, clang complains about - dnl use of --param=ssp-buffer-size=4 unless used with - dnl the -c arg. It doesn't like it when used with args - dnl that just link together .o files. Unfortunately - dnl we can't avoid that with automake, so we must turn - dnl off the following clang specific warning - wantwarn="$wantwarn -Wno-unused-command-line-argument"
Why do you also remove this line?
;; *-*-freebsd*) dnl FreeBSD ships old gcc 4.2.1 which doesn't handle
Also, out of the context of this patch, doesn't that param need to be added to the freebsd version since it uses -fstack-protector only? Martin
On Fri, Jun 06, 2014 at 01:00:20PM +0200, Martin Kletzander wrote:
On Fri, Jun 06, 2014 at 11:40:24AM +0200, Ján Tomko wrote:
This option only makes sense with -fstack-protector. With -fstack-protector-all, even functions with buffers smaller than this are protected.
https://bugzilla.redhat.com/show_bug.cgi?id=1105456 --- m4/virt-compile-warnings.m4 | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 574fbc4..ebc931d 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -171,14 +171,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl be great overhead in adding -fstack-protector-all instead dnl wantwarn="$wantwarn -fstack-protector" wantwarn="$wantwarn -fstack-protector-all" - wantwarn="$wantwarn --param=ssp-buffer-size=4"
It would be nice to keep that line in here with the explanation that -fstack-protector-all does not make use of that param.
- dnl Even though it supports it, clang complains about - dnl use of --param=ssp-buffer-size=4 unless used with - dnl the -c arg. It doesn't like it when used with args - dnl that just link together .o files. Unfortunately - dnl we can't avoid that with automake, so we must turn - dnl off the following clang specific warning - wantwarn="$wantwarn -Wno-unused-command-line-argument"
Why do you also remove this line?
;; *-*-freebsd*) dnl FreeBSD ships old gcc 4.2.1 which doesn't handle
Also, out of the context of this patch, doesn't that param need to be added to the freebsd version since it uses -fstack-protector only?
Ideally we should actually use -fstack-protector-strong if we find it supported, in preference to -fstack-protector-all. The strong variant would still require us to set ssp-buffer-size. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 06/06/2014 01:03 PM, Daniel P. Berrange wrote:
On Fri, Jun 06, 2014 at 01:00:20PM +0200, Martin Kletzander wrote:
On Fri, Jun 06, 2014 at 11:40:24AM +0200, Ján Tomko wrote:
This option only makes sense with -fstack-protector. With -fstack-protector-all, even functions with buffers smaller than this are protected.
https://bugzilla.redhat.com/show_bug.cgi?id=1105456 --- m4/virt-compile-warnings.m4 | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 574fbc4..ebc931d 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -171,14 +171,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl be great overhead in adding -fstack-protector-all instead dnl wantwarn="$wantwarn -fstack-protector" wantwarn="$wantwarn -fstack-protector-all" - wantwarn="$wantwarn --param=ssp-buffer-size=4"
It would be nice to keep that line in here with the explanation that -fstack-protector-all does not make use of that param.
On second thought, the buffer size makes sense for -fstack-protector, so I guess it should stay unless we remove '-fstack-protector' as well.
- dnl Even though it supports it, clang complains about - dnl use of --param=ssp-buffer-size=4 unless used with - dnl the -c arg. It doesn't like it when used with args - dnl that just link together .o files. Unfortunately - dnl we can't avoid that with automake, so we must turn - dnl off the following clang specific warning - wantwarn="$wantwarn -Wno-unused-command-line-argument"
Why do you also remove this line?
This warning supression was only added because of the --param flag, which I was removing (see the comment above it).
;; *-*-freebsd*) dnl FreeBSD ships old gcc 4.2.1 which doesn't handle
Also, out of the context of this patch, doesn't that param need to be added to the freebsd version since it uses -fstack-protector only?
I can't do any proper testing on FreeBSD, maybe it would work better than stack-protector-all with 4.2.1: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=cc7cd623
Ideally we should actually use -fstack-protector-strong if we find it supported, in preference to -fstack-protector-all.
That could work, if the GCC version shipped on FreeBSD doesn't have it broken. I can send a patch enabling it for Linux after I upgrade my compiler.
The strong variant would still require us to set ssp-buffer-size.
Not really, gcc only uses it to tell small and large arrays apart: https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/cfgexpand.c?revision=211306&view=markup#l1391 and then treats them the same for stack-protector-all and -strong: https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/cfgexpand.c?revision=211306&view=markup#l1439 Jan
On 06.06.2014 11:40, Ján Tomko wrote:
This option only makes sense with -fstack-protector. With -fstack-protector-all, even functions with buffers smaller than this are protected.
https://bugzilla.redhat.com/show_bug.cgi?id=1105456 --- m4/virt-compile-warnings.m4 | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 574fbc4..ebc931d 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -171,14 +171,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl be great overhead in adding -fstack-protector-all instead dnl wantwarn="$wantwarn -fstack-protector" wantwarn="$wantwarn -fstack-protector-all" - wantwarn="$wantwarn --param=ssp-buffer-size=4" - dnl Even though it supports it, clang complains about - dnl use of --param=ssp-buffer-size=4 unless used with - dnl the -c arg. It doesn't like it when used with args - dnl that just link together .o files. Unfortunately - dnl we can't avoid that with automake, so we must turn - dnl off the following clang specific warning - wantwarn="$wantwarn -Wno-unused-command-line-argument" ;; *-*-freebsd*) dnl FreeBSD ships old gcc 4.2.1 which doesn't handle
From the gcc man page: -fstack-protector Emit extra code to check for buffer overflows, such as stack smashing attacks. This is done by adding a guard variable to functions with vulnerable objects. This includes functions that call "alloca", and functions with buffers larger than 8 bytes. The guards are initialized when a function is entered and then checked when the function exits. If a guard check fails, an error message is printed and the program exits. -fstack-protector-all Like -fstack-protector except that all functions are protected. So when using -fstack-protector-all even functions with 4B buffers are protected. ACK Michal
participants (4)
-
Daniel P. Berrange -
Ján Tomko -
Martin Kletzander -
Michal Privoznik