linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* propolice support for linux
@ 2005-01-13 13:46 Han Boetes
  2005-01-13 14:04 ` Christoph Hellwig
                   ` (4 more replies)
  0 siblings, 5 replies; 23+ messages in thread
From: Han Boetes @ 2005-01-13 13:46 UTC (permalink / raw)
  To: linux-kernel

Hi,

The propolice gcc-extension prevents buffer-overflows in binaries:

  http://www.research.ibm.com/trl/projects/security/ssp/

The effect is that all buffer-overflow exploits are turned into a
-- logged -- Denial of service.

And since most of the security-flaws in linux are buffer-overflows
I would like to request that a patch based on this one is applied
to the kernel so people can use this extension by default.


Note: The propolice-patch for gcc-3.3.2 also applies fine to
      gcc-3.3.5
Note: glibc from CVS already supports propolice.
Note: OpenBSD is fully compiled with propolice.


  http://frogger974.homelinux.org/propolice/linux-2.6.3-ssp-config-1.patch

diff -urN linux-2.6.3/Makefile linux-2.6.3.ssp/Makefile
--- linux-2.6.3/Makefile	2004-02-17 22:58:39.000000000 -0500
+++ linux-2.6.3.ssp/Makefile	2004-03-03 10:20:29.000000000 -0500
@@ -442,6 +442,10 @@
 CFLAGS		+= -fomit-frame-pointer
 endif
 
+ifdef CONFIG_HARDENED_SSP
+CFLAGS += -fstack-protector
+endif
+
 ifdef CONFIG_DEBUG_INFO
 CFLAGS		+= -g
 endif
diff -urN linux-2.6.3/include/linux/kernel.h linux-2.6.3.ssp/include/linux/kernel.h
--- linux-2.6.3/include/linux/kernel.h	2004-02-17 22:57:11.000000000 -0500
+++ linux-2.6.3.ssp/include/linux/kernel.h	2004-03-03 10:08:10.000000000 -0500
@@ -115,6 +115,10 @@
 #define TAINT_FORCED_RMMOD		(1<<3)
 
 extern void dump_stack(void);
+#ifdef CONFIG_HARDENED_SSP
+extern int __guard;
+extern void __stack_smash_handler(int, char []);
+#endif
 
 #ifdef DEBUG
 #define pr_debug(fmt,arg...) \
Files linux-2.6.3/lib/.propolice.c.swp and linux-2.6.3.ssp/lib/.propolice.c.swp differ
diff -urN linux-2.6.3/lib/Makefile linux-2.6.3.ssp/lib/Makefile
--- linux-2.6.3/lib/Makefile	2004-02-17 22:57:14.000000000 -0500
+++ linux-2.6.3.ssp/lib/Makefile	2004-03-03 13:47:27.000000000 -0500
@@ -20,6 +20,8 @@
 obj-$(CONFIG_ZLIB_INFLATE) += zlib_inflate/
 obj-$(CONFIG_ZLIB_DEFLATE) += zlib_deflate/
 
+obj-$(CONFIG_HARDENED_SSP) += propolice.o
+
 host-progs	:= gen_crc32table
 clean-files	:= crc32table.h
 
diff -urN linux-2.6.3/lib/propolice.c linux-2.6.3.ssp/lib/propolice.c
--- linux-2.6.3/lib/propolice.c	1969-12-31 19:00:00.000000000 -0500
+++ linux-2.6.3.ssp/lib/propolice.c	2004-03-03 17:52:48.000000000 -0500
@@ -0,0 +1,15 @@
+#include <linux/module.h>
+#include <linux/errno.h>
+
+EXPORT_SYMBOL_NOVERS(__guard);
+EXPORT_SYMBOL_NOVERS(__stack_smash_handler);
+ 
+int __guard = '\0\0\n\777';
+ 
+void 
+__stack_smash_handler (int damaged, char func[])
+{
+	static char *message = "propolice detects %x at function %s.\n" ;
+	panic (message, damaged, func);
+}
+
diff -urN linux-2.6.3/security/Kconfig linux-2.6.3.ssp/security/Kconfig
--- linux-2.6.3/security/Kconfig	2004-02-17 22:58:44.000000000 -0500
+++ linux-2.6.3.ssp/security/Kconfig	2004-03-03 13:50:30.000000000 -0500
@@ -46,5 +46,11 @@
 
 source security/selinux/Kconfig
 
+config HARDENED_SSP
+	bool 'Hardened ProPolice SSP build support'
+	help
+	  This enables kernel building with stack-smashing protection
+	  via the -fstack-protector GCC flag.
+
 endmenu
 


# Han

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 13:46 propolice support for linux Han Boetes
@ 2005-01-13 14:04 ` Christoph Hellwig
  2005-01-13 14:53   ` Arjan van de Ven
  2005-01-13 16:37   ` Han Boetes
  2005-01-13 14:07 ` Arjan van de Ven
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 23+ messages in thread
From: Christoph Hellwig @ 2005-01-13 14:04 UTC (permalink / raw)
  To: linux-kernel

> --- linux-2.6.3/Makefile	2004-02-17 22:58:39.000000000 -0500
> +++ linux-2.6.3.ssp/Makefile	2004-03-03 10:20:29.000000000 -0500

2.6.3 is from stoneage..

> +ifdef CONFIG_HARDENED_SSP
> +CFLAGS += -fstack-protector
> +endif

What about just CONFIG_PROPOLICY?  The hardnedefoo naming is so childish
(or gentooish, in the end it's the same anyway..)

> diff -urN linux-2.6.3/include/linux/kernel.h linux-2.6.3.ssp/include/linux/kernel.h
> --- linux-2.6.3/include/linux/kernel.h	2004-02-17 22:57:11.000000000 -0500
> +++ linux-2.6.3.ssp/include/linux/kernel.h	2004-03-03 10:08:10.000000000 -0500
> @@ -115,6 +115,10 @@
>  #define TAINT_FORCED_RMMOD		(1<<3)
>  
>  extern void dump_stack(void);
> +#ifdef CONFIG_HARDENED_SSP
> +extern int __guard;
> +extern void __stack_smash_handler(int, char []);
> +#endif

What do you need these prototypes for, they're not used at all.  Also
no need to put ifdefs around them.

> diff -urN linux-2.6.3/lib/propolice.c linux-2.6.3.ssp/lib/propolice.c
> --- linux-2.6.3/lib/propolice.c	1969-12-31 19:00:00.000000000 -0500
> +++ linux-2.6.3.ssp/lib/propolice.c	2004-03-03 17:52:48.000000000 -0500
> @@ -0,0 +1,15 @@
> +#include <linux/module.h>
> +#include <linux/errno.h>

What do you need errno for?

> 
> +
> +EXPORT_SYMBOL_NOVERS(__guard);
> +EXPORT_SYMBOL_NOVERS(__stack_smash_handler);
> + 
> +int __guard = '\0\0\n\777';
> + 
> +void 
> +__stack_smash_handler (int damaged, char func[])
> +{
> +	static char *message = "propolice detects %x at function %s.\n" ;
> +	panic (message, damaged, func);
> +}

ah, it seems you need them because you put the exports before the
delaration.  This file should probably look more like:

/*
 * Insert Copyright here.
 *
 * Insert small description here.
 */
#include <linux/kernel.h>
#include <linux/module.h>

int __guard = '\0\0\n\777';
EXPORT_SYMBOL_NOVERS(__guard);
 
static const char message[] = "propolice detects %x at function %s.\n";

void __stack_smash_handler(int damaged, char func[])
{
	panic(message, damaged, func);
}
EXPORT_SYMBOL_NOVERS(__stack_smash_handler);



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 13:46 propolice support for linux Han Boetes
  2005-01-13 14:04 ` Christoph Hellwig
@ 2005-01-13 14:07 ` Arjan van de Ven
  2005-01-13 14:15 ` Jakub Jelinek
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 23+ messages in thread
From: Arjan van de Ven @ 2005-01-13 14:07 UTC (permalink / raw)
  To: Han Boetes; +Cc: linux-kernel

On Thu, 2005-01-13 at 14:45 +0059, Han Boetes wrote:

> And since most of the security-flaws in linux are buffer-overflows
> I would like to request that a patch based on this one is applied
> to the kernel so people can use this extension by default.
> 

I'm sorry but I disagree with this. Most of the security flaws in the
kernel are NOT buffer overflows. Almost none are! (and that is because
in the linux kernel you are very much stack constrained and can't put
large-ish buffers on the stack).

Userland.. that's a different matter.
Propolice is one of the options there, there are others too. But for the
kernel, buffer overflows are really rare (esp ones that propolice and
other tools can catch).



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 13:46 propolice support for linux Han Boetes
  2005-01-13 14:04 ` Christoph Hellwig
  2005-01-13 14:07 ` Arjan van de Ven
@ 2005-01-13 14:15 ` Jakub Jelinek
  2005-01-13 19:58 ` Andi Kleen
  2005-01-13 21:11 ` Ulrich Drepper
  4 siblings, 0 replies; 23+ messages in thread
From: Jakub Jelinek @ 2005-01-13 14:15 UTC (permalink / raw)
  To: linux-kernel

On Thu, Jan 13, 2005 at 02:45:58PM +0059, Han Boetes wrote:
> Hi,
> 
> The propolice gcc-extension prevents buffer-overflows in binaries:
> 
>   http://www.research.ibm.com/trl/projects/security/ssp/
> 
> The effect is that all buffer-overflow exploits are turned into a
> -- logged -- Denial of service.

This is little bit too strong.
The effect is that it detects some stack buffer-overflow exploits.

	Jakub

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 14:04 ` Christoph Hellwig
@ 2005-01-13 14:53   ` Arjan van de Ven
  2005-01-13 16:37   ` Han Boetes
  1 sibling, 0 replies; 23+ messages in thread
From: Arjan van de Ven @ 2005-01-13 14:53 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-kernel


> EXPORT_SYMBOL_NOVERS(__guard);

_NOVERS is dead btw


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 14:04 ` Christoph Hellwig
  2005-01-13 14:53   ` Arjan van de Ven
@ 2005-01-13 16:37   ` Han Boetes
  2005-01-13 17:02     ` Arjan van de Ven
                       ` (3 more replies)
  1 sibling, 4 replies; 23+ messages in thread
From: Han Boetes @ 2005-01-13 16:37 UTC (permalink / raw)
  To: linux-kernel


Thanks for your comments! This wasn't my patch, just the closed
thing to something working I could find. Here is my version.

Now all I wonder about is what the _NOVERS should become, since
Arjen pointed it it `was dead,' since I don't really understand
what he means with that.

Perhaps I should also add some additional comment about how little
effect this extension resorts on kernel-level.

And I got two warnings about `int __guard = '\0\0\n\777';'

lib/propolice.c:15:15: warning: octal escape sequence out of range
lib/propolice.c:15:15: warning: multi-character character constant



--- linux-2.6.9/lib/propolice.c.orig	2005-01-13 17:08:49.920963760 +0100
+++ linux-2.6.9/lib/propolice.c	2005-01-13 16:46:48.939783424 +0100
@@ -0,0 +1,24 @@
+/*
+ * Copyright 2005, Han Boetes <han@boetes.org>
+ *
+ * This code adds support for the propolice stacksmashing
+ * extension for gcc.
+ * http://www.research.ibm.com/trl/projects/security/ssp/
+ *
+ * This source code is licensed under the GNU General Public
+ * License, Version 2. See the file COPYING for more details.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+int __guard = '\0\0\n\777';
+EXPORT_SYMBOL_NOVERS(__guard);
+
+static const char message[] = "propolice detects %x at function %s.\n";
+
+void __stack_smash_handler(int damaged, char func[])
+{
+    panic(message, damaged, func);
+}
+EXPORT_SYMBOL_NOVERS(__stack_smash_handler);
--- linux-2.6.9/lib/Makefile.orig	2005-01-13 16:47:58.564198904 +0100
+++ linux-2.6.9/lib/Makefile	2005-01-13 17:06:29.124368096 +0100
@@ -23,6 +23,8 @@ obj-$(CONFIG_GENERIC_IOMAP) += iomap.o
 obj-$(CONFIG_ZLIB_INFLATE) += zlib_inflate/
 obj-$(CONFIG_ZLIB_DEFLATE) += zlib_deflate/
 
+obj-$(CONFIG_PROPOLICE) += propolice.o
+
 hostprogs-y	:= gen_crc32table
 clean-files	:= crc32table.h
 
--- linux-2.6.9/security/Kconfig.orig	2004-10-18 23:54:39.000000000 +0200
+++ linux-2.6.9/security/Kconfig	2005-01-13 16:57:23.130371800 +0100
@@ -44,6 +44,18 @@ config SECURITY_ROOTPLUG
 	  
 	  If you are unsure how to answer this question, answer N.
 
+config PROPOLICE
+       bool 'GCC ProPolice SSP build support'
+       help
+         This enables kernel building with stack-smashing protection
+         via the -fstack-protector GCC flag, if you have GCC build with
+	 propolice.
+
+	 See <http://www.research.ibm.com/trl/projects/security/ssp/> for
+	 more information about this compiler-extension.
+
+	 If you are unsure how to answer this question, answer N.
+
 source security/selinux/Kconfig
 
 endmenu
--- linux-2.6.9/Makefile.orig	2005-01-13 16:38:39.479192744 +0100
+++ linux-2.6.9/Makefile	2005-01-13 16:40:45.139089536 +0100
@@ -490,6 +490,10 @@ ifndef CONFIG_FRAME_POINTER
 CFLAGS		+= -fomit-frame-pointer
 endif
 
+ifdef CONFIG_PROPOLICE
+CFLAGS		+= -fstack-protector
+endif
+
 ifdef CONFIG_DEBUG_INFO
 CFLAGS		+= -g
 endif




# Han

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 16:37   ` Han Boetes
@ 2005-01-13 17:02     ` Arjan van de Ven
  2005-01-13 19:32       ` Han Boetes
  2005-01-13 17:07     ` Bill Davidsen
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 23+ messages in thread
From: Arjan van de Ven @ 2005-01-13 17:02 UTC (permalink / raw)
  To: Han Boetes; +Cc: linux-kernel

On Thu, 2005-01-13 at 17:37 +0100, Han Boetes wrote:
> Thanks for your comments! This wasn't my patch, just the closed
> thing to something working I could find. Here is my version.


you haven't commented on the "why is this useful for the kernel"
question I asked, can you point out a single kernel buffer overflow ??

(I know userspace has them; I'm asking you specifically about kernel
space since you provide a patch useful for kernel space only)


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 16:37   ` Han Boetes
  2005-01-13 17:02     ` Arjan van de Ven
@ 2005-01-13 17:07     ` Bill Davidsen
  2005-01-13 17:31       ` Han Boetes
  2005-01-13 17:58     ` Zwane Mwaikambo
  2005-01-14  4:25     ` Mitchell Blank Jr
  3 siblings, 1 reply; 23+ messages in thread
From: Bill Davidsen @ 2005-01-13 17:07 UTC (permalink / raw)
  To: linux-kernel, Han Boetes; +Cc: linux-kernel

Han Boetes wrote:
> Thanks for your comments! This wasn't my patch, just the closed
> thing to something working I could find. Here is my version.
> 
> Now all I wonder about is what the _NOVERS should become, since
> Arjen pointed it it `was dead,' since I don't really understand
> what he means with that.
> 
> Perhaps I should also add some additional comment about how little
> effect this extension resorts on kernel-level.
> 
> And I got two warnings about `int __guard = '\0\0\n\777';'
> 
> lib/propolice.c:15:15: warning: octal escape sequence out of range
> lib/propolice.c:15:15: warning: multi-character character constant

Unless you foresee a port of Linux to some 36 bit hardware (like 
MULTICS) with nine bit bytes, is there a reason not to us \377? I have 
used 36 (and 48) bit hardware, but I don't expect it to ever get a Linux 
port.

-- 
    -bill davidsen (davidsen@tmr.com)
"The secret to procrastination is to put things off until the
  last possible moment - but no longer"  -me

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 17:07     ` Bill Davidsen
@ 2005-01-13 17:31       ` Han Boetes
  0 siblings, 0 replies; 23+ messages in thread
From: Han Boetes @ 2005-01-13 17:31 UTC (permalink / raw)
  To: linux-kernel

Bill Davidsen wrote:
> Han Boetes wrote:
> > And I got two warnings about `int __guard = '\0\0\n\777';'
> >
> > lib/propolice.c:15:15: warning: octal escape sequence out of range
> > lib/propolice.c:15:15: warning: multi-character character constant
>
> Unless you foresee a port of Linux to some 36 bit hardware (like 
> MULTICS) with nine bit bytes, is there a reason not to us \377? I have 
> used 36 (and 48) bit hardware, but I don't expect it to ever get a Linux 
> port.

Could you please refrain from using rhetorical questions, since
they really obscure what you are trying explain and only appear to
intend to embarrass me.

If I understand right what you just said I would suggest you would
have say something like: ``This should most likely be \377. \777
is intended for 36 bit hardware.''



# Han

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 16:37   ` Han Boetes
  2005-01-13 17:02     ` Arjan van de Ven
  2005-01-13 17:07     ` Bill Davidsen
@ 2005-01-13 17:58     ` Zwane Mwaikambo
  2005-01-13 18:17       ` Han Boetes
  2005-01-14  4:25     ` Mitchell Blank Jr
  3 siblings, 1 reply; 23+ messages in thread
From: Zwane Mwaikambo @ 2005-01-13 17:58 UTC (permalink / raw)
  To: Han Boetes; +Cc: linux-kernel

On Thu, 13 Jan 2005, Han Boetes wrote:

> 
> Thanks for your comments! This wasn't my patch, just the closed
> thing to something working I could find. Here is my version.
> 
> Now all I wonder about is what the _NOVERS should become, since
> Arjen pointed it it `was dead,' since I don't really understand
> what he means with that.

Just use the normal EXPORT_SYMBOL it has the same effect.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 17:58     ` Zwane Mwaikambo
@ 2005-01-13 18:17       ` Han Boetes
  0 siblings, 0 replies; 23+ messages in thread
From: Han Boetes @ 2005-01-13 18:17 UTC (permalink / raw)
  To: linux-kernel

Zwane Mwaikambo wrote:
> On Thu, 13 Jan 2005, Han Boetes wrote:
> > Now all I wonder about is what the _NOVERS should become, since
> > Arjen pointed it it `was dead,' since I don't really understand
> > what he means with that.
>
> Just use the normal EXPORT_SYMBOL it has the same effect.

Thank you, much appreciated.

Here is the latest version of the patch:

--- linux-2.6.9/lib/propolice.c.orig	2005-01-13 17:08:49.920963760 +0100
+++ linux-2.6.9/lib/propolice.c	2005-01-13 16:46:48.939783424 +0100
@@ -0,0 +1,24 @@
+/*
+ * Copyright 2005, Han Boetes <han@boetes.org>
+ *
+ * This code adds support for the propolice stacksmashing
+ * extension for gcc.
+ * http://www.research.ibm.com/trl/projects/security/ssp/
+ *
+ * This source code is licensed under the GNU General Public
+ * License, Version 2. See the file COPYING for more details.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+int __guard = '\0\0\n\377';
+EXPORT_SYMBOL(__guard);
+
+static const char message[] = "propolice detects %x at function %s.\n";
+
+void __stack_smash_handler(int damaged, char func[])
+{
+    panic(message, damaged, func);
+}
+EXPORT_SYMBOL(__stack_smash_handler);
--- linux-2.6.9/lib/Makefile.orig	2005-01-13 16:47:58.564198904 +0100
+++ linux-2.6.9/lib/Makefile	2005-01-13 17:06:29.124368096 +0100
@@ -23,6 +23,8 @@ obj-$(CONFIG_GENERIC_IOMAP) += iomap.o
 obj-$(CONFIG_ZLIB_INFLATE) += zlib_inflate/
 obj-$(CONFIG_ZLIB_DEFLATE) += zlib_deflate/
 
+obj-$(CONFIG_PROPOLICE) += propolice.o
+
 hostprogs-y	:= gen_crc32table
 clean-files	:= crc32table.h
 
--- linux-2.6.9/security/Kconfig.orig	2004-10-18 23:54:39.000000000 +0200
+++ linux-2.6.9/security/Kconfig	2005-01-13 16:57:23.130371800 +0100
@@ -44,6 +44,18 @@ config SECURITY_ROOTPLUG
 	  
 	  If you are unsure how to answer this question, answer N.
 
+config PROPOLICE
+       bool 'GCC ProPolice SSP build support'
+       help
+         This enables kernel building with stack-smashing protection
+         via the -fstack-protector GCC flag, if you have GCC build with
+	 propolice.
+
+	 See <http://www.research.ibm.com/trl/projects/security/ssp/> for
+	 more information about this compiler-extension.
+
+	 If you are unsure how to answer this question, answer N.
+
 source security/selinux/Kconfig
 
 endmenu
--- linux-2.6.9/Makefile.orig	2005-01-13 16:38:39.479192744 +0100
+++ linux-2.6.9/Makefile	2005-01-13 16:40:45.139089536 +0100
@@ -490,6 +490,10 @@ ifndef CONFIG_FRAME_POINTER
 CFLAGS		+= -fomit-frame-pointer
 endif
 
+ifdef CONFIG_PROPOLICE
+CFLAGS		+= -fstack-protector
+endif
+
 ifdef CONFIG_DEBUG_INFO
 CFLAGS		+= -g
 endif




# Han

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 17:02     ` Arjan van de Ven
@ 2005-01-13 19:32       ` Han Boetes
  2005-01-14  7:35         ` Arjan van de Ven
  0 siblings, 1 reply; 23+ messages in thread
From: Han Boetes @ 2005-01-13 19:32 UTC (permalink / raw)
  To: linux-kernel

Arjan van de Ven wrote:
> you haven't commented on the "why is this useful for the kernel"
> question I asked,

You didn't ask this question, and I did comment.


> can you point out a single kernel buffer overflow ??

Let's ask Edsger Dijkstra :-)

  ``Program testing can be used to show the presence of bugs, but
    never to show their absence! -- Edsger Dijkstra, [1972]''

I get your point though, but since it can't do any harm let us
paranoid freaks have a proper patch at the least.


> (I know userspace has them; I'm asking you specifically about
> kernel space since you provide a patch useful for kernel space
> only)


Well I suggest it here as well:
  http://forums.mozillazine.org/viewtopic.php?t=199891

But got zero replies. Could you be so kind to point out to these
people it's a good idea?



# Han

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 13:46 propolice support for linux Han Boetes
                   ` (2 preceding siblings ...)
  2005-01-13 14:15 ` Jakub Jelinek
@ 2005-01-13 19:58 ` Andi Kleen
  2005-01-13 21:11 ` Ulrich Drepper
  4 siblings, 0 replies; 23+ messages in thread
From: Andi Kleen @ 2005-01-13 19:58 UTC (permalink / raw)
  To: linux-kernel; +Cc: han

Han Boetes <han@mijncomputer.nl> writes:

> And since most of the security-flaws in linux are buffer-overflows
> I would like to request that a patch based on this one is applied
> to the kernel so people can use this extension by default.

Not in the linux kernel. If you followed the last security issues
none of them involved a overrun stack buffer. In fact I can only
remember one stack buffer overflow at all in the kernel long ago.

-Andi


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 13:46 propolice support for linux Han Boetes
                   ` (3 preceding siblings ...)
  2005-01-13 19:58 ` Andi Kleen
@ 2005-01-13 21:11 ` Ulrich Drepper
  2005-01-13 22:52   ` Han Boetes
  4 siblings, 1 reply; 23+ messages in thread
From: Ulrich Drepper @ 2005-01-13 21:11 UTC (permalink / raw)
  To: linux-kernel

Aside from all the arguments about why this patch isn't good for the
kernel, everybody should be aware that the ProPolice gcc patches are
pretty unusable.  They rely in recognizing certain tree patterns which
for some architectures do not exist, and for others can look
differently, depending on the optimization.  To paraphrase one of the
gcc developers: "this kind of functionality should be written to work
_with_ gcc, not _against_ it as the propolice patch does".

Before you suggest using something like this patch you better first
inform yourself by asking the people who actually know the code which
is modified.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 21:11 ` Ulrich Drepper
@ 2005-01-13 22:52   ` Han Boetes
  2005-01-14  6:25     ` Willy Tarreau
  2005-01-14  7:06     ` Ulrich Drepper
  0 siblings, 2 replies; 23+ messages in thread
From: Han Boetes @ 2005-01-13 22:52 UTC (permalink / raw)
  To: linux-kernel

Ulrich Drepper wrote:
> Aside from all the arguments about why this patch isn't good for
> the kernel, everybody should be aware that the ProPolice gcc
> patches are pretty unusable. They rely in recognizing certain
> tree patterns which for some architectures do not exist, and for
> others can look differently, depending on the optimization. To
> paraphrase one of the gcc developers: "this kind of
> functionality should be written to work _with_ gcc, not
> _against_ it as the propolice patch does".
>
> Before you suggest using something like this patch you better
> first inform yourself by asking the people who actually know the
> code which is modified.

Ok I have a problem here. You are Ulrich Drepper and you are _the_
maintainer of glibc and I am Han Boetes and I am a C noob. It's
like Kasparov trying to explain something to a club-chess-player.

I'm afraid that whatever explanation you give to me is over my
head, you'll feel like talking to a dummy and I'll be terribly
unsatisfied.

To avoid that I would like to ask you if you can show me some
example-code, something I which can compile and run and see for
myself, for the following situations:

1) Where an application compiled with PP is working worse or even
   failing where it would work right without PP.

2) Where a bufferoverflow can be exploited even though the
   application is compiled with PP.


As an example where PP does work right the test-code provided by
the propolice maintainer:

    /* test-propolice.c */
    #define OVERFLOW "This is longer than 10 bytes"
    #include <string.h>
    int main (int argc, char *argv[]) {
        char buffer[10];
        strcpy(buffer, OVERFLOW);
        return 0;
    }




# Han

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 16:37   ` Han Boetes
                       ` (2 preceding siblings ...)
  2005-01-13 17:58     ` Zwane Mwaikambo
@ 2005-01-14  4:25     ` Mitchell Blank Jr
  2005-01-14 10:30       ` Han Boetes
  3 siblings, 1 reply; 23+ messages in thread
From: Mitchell Blank Jr @ 2005-01-14  4:25 UTC (permalink / raw)
  To: Han Boetes; +Cc: linux-kernel

Han Boetes wrote:
> And I got two warnings about `int __guard = '\0\0\n\777';'
> 
> lib/propolice.c:15:15: warning: octal escape sequence out of range

Yeah, I have no idea what '\777' is supposed to be - that's 9 bits wide.

> lib/propolice.c:15:15: warning: multi-character character constant

Since the __guard symbol isn't used directly by C code it doesn't matter
what type it is, right?  You should be able to just say something like:

unsigned char __guard[sizeof(int)] = { '\0', '\0', '\n', (unsighed char) -1 };

Or maybe you want those in the opposite order for endianness.. not sure.
Having the '\0' first in memory makes the most sense to me but I haven't
thought about it much.

> +static const char message[] = "propolice detects %x at function %s.\n";
> +
> +void __stack_smash_handler(int damaged, char func[])

1. "damaged" should be "unsigned int"; it's used with a "%x" format specifier
2. "func" should probably be "const char *func"
3. Why is message[] defined instead of just using it directly in the call
   to panic like panic("blah blah...", ...)  Maybe I'm missing something
   subtle there.

-Mitch

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 22:52   ` Han Boetes
@ 2005-01-14  6:25     ` Willy Tarreau
  2005-01-14  7:06     ` Ulrich Drepper
  1 sibling, 0 replies; 23+ messages in thread
From: Willy Tarreau @ 2005-01-14  6:25 UTC (permalink / raw)
  To: linux-kernel

Hi,

On Thu, Jan 13, 2005 at 11:52:22PM +0100, Han Boetes wrote:
> 1) Where an application compiled with PP is working worse or even
>    failing where it would work right without PP.

No idea on this one, I never tried PP, although I know how it basically
works since I worked on a similar concept in 97 of last century (but I
didn't have the skills to touch the compiler and still don't).

> 2) Where a bufferoverflow can be exploited even though the
>    application is compiled with PP.

1) any broken function of this kind :

int create_temp_dir(struct task *t, char *dir) {
   int err;
   int can_unlink;
   char dirname[MAXPATHLEN];

   can_unlink = (task->euid == 0);
   strncpy(dirname, dir, MAXPATHLEN);
   strcat(dirname, "/tmp");
   dirname[MAXPATHLEN] = '\0';

   if (err = mkdir(dirname, 0755)) {
      if (can_unlink) {
         unlink(dirname);
         err = mkdir(dirname, 0755);
      }
   }
   return err;
}

Get it ? Just pass any name of MAXPATHLEN length, and get
any existing file removed and replace with an empty directory.
Useful for hosts.deny, /var/log/messages, init scripts, etc...

2) all heap overflows (but not in kernel AFAIK).

I think you have a misconception about what a buffer overflow is. First,
propolice will be usable only against some *stack* overflows (which I agree
are the most common in userspace). But regular buffer overflows like above,
which can be triggered either in the stack on in the data space, are not
stopped. And heap overflows such as the double free bug in zlib will not
be prevented by propolice either.

> As an example where PP does work right the test-code provided by
> the propolice maintainer:
> 
>     /* test-propolice.c */
>     #define OVERFLOW "This is longer than 10 bytes"
>     #include <string.h>
>     int main (int argc, char *argv[]) {
>         char buffer[10];
>         strcpy(buffer, OVERFLOW);
>         return 0;
>     }
> 

This is kind, but this is also the easiest buggy program to write. The one
we all use when trying to write shell code. But this is far away from real
life, there often are other constraints.

Like others, I think that PP is close to useless in the kernel, but since
the patch is little and does not break anything, why not include it to let
people try it and return feedback ?

Regards,
Willy


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 22:52   ` Han Boetes
  2005-01-14  6:25     ` Willy Tarreau
@ 2005-01-14  7:06     ` Ulrich Drepper
  2005-01-14 14:08       ` Nix
  1 sibling, 1 reply; 23+ messages in thread
From: Ulrich Drepper @ 2005-01-14  7:06 UTC (permalink / raw)
  To: linux-kernel

On Thu, 13 Jan 2005 23:52:22 +0100, Han Boetes <han@mijncomputer.nl> wrote:
> To avoid that I would like to ask you if you can show me some
> example-code, something I which can compile and run and see for
> myself, for the following situations:

The analysis of the patch does not stem from trying programs and
seeing them failing.  The gcc maintainers looked at it with
understanding of the compiler and the way the patch interacts with the
existing code.  It might be possible for them to come up with a
program which is miscompiled.  More importantly, though, it'll most
probably be possible to come up with code which should be instrumented
but isn't.  You must agree that this is a terrible thing to happen. 
The patch gives a wrong sense of security.

Finally, the gcc patch is not going to work as is on architectures
like IA-64 which do not have the kind of adressing modes which are
needed for the code to work.

To fully understand the problem, you need to understand compiler
design, and especially RTL.  The latter by itself is another problem:
getting the code work in gcc 4 is at least challenging due the SSA.

Anyway, if you want more information you'll need to ask the gcc people.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-13 19:32       ` Han Boetes
@ 2005-01-14  7:35         ` Arjan van de Ven
  0 siblings, 0 replies; 23+ messages in thread
From: Arjan van de Ven @ 2005-01-14  7:35 UTC (permalink / raw)
  To: Han Boetes; +Cc: linux-kernel


> Well I suggest it here as well:
>   http://forums.mozillazine.org/viewtopic.php?t=199891
> 
> But got zero replies. Could you be so kind to point out to these
> people it's a good idea?


there are other alternatives to PP that don't require code changes; for
example Jakub wrote a patch for gcc4 and 3.4 that has the effect of
detecting buffer overflows but that
1) Does not require code changes in userspace (only CFLAGS)
2) is supposedly better in line with the gcc architecture (but I'm not a
gcc export to judge that)

Before people say "NIH NIH NIH" I have to say this: Propolice is a great
piece of work. But if it's architecture is not suitable for gcc mainline
(especially with the SSA changes in gcc4) then it's most likely not
going anywhere (as a way of "proof": PP is around for a long time, but
it still hasn't made gcc mainline, I don't even know if it has been
submitted for that even). The functionality of detecting buffer
overflows is useful, and Jakubs patch addresses that functionality as
well, but in a way that is more in line with the gcc design. 
(It's actually quite cool stuff, current rawhide is getting compiled
with it for example and working quite nicely there)



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-14  4:25     ` Mitchell Blank Jr
@ 2005-01-14 10:30       ` Han Boetes
  2005-01-15  2:25         ` Mitchell Blank Jr
  0 siblings, 1 reply; 23+ messages in thread
From: Han Boetes @ 2005-01-14 10:30 UTC (permalink / raw)
  To: linux-kernel

Thanks for your excellent comments. However unimportant this patch
is for kernel-security at least lets make it an exercise in decent
coding :-)

--- linux-2.6.9/lib/Makefile.orig	2005-01-13 16:47:58.564198904 +0100
+++ linux-2.6.9/lib/Makefile	2005-01-13 17:06:29.124368096 +0100
@@ -23,6 +23,8 @@ obj-$(CONFIG_GENERIC_IOMAP) += iomap.o
 obj-$(CONFIG_ZLIB_INFLATE) += zlib_inflate/
 obj-$(CONFIG_ZLIB_DEFLATE) += zlib_deflate/
 
+obj-$(CONFIG_PROPOLICE) += propolice.o
+
 hostprogs-y	:= gen_crc32table
 clean-files	:= crc32table.h
 
--- linux-2.6.9/security/Kconfig.orig	2004-10-18 23:54:39.000000000 +0200
+++ linux-2.6.9/security/Kconfig	2005-01-13 16:57:23.130371800 +0100
@@ -44,6 +44,18 @@ config SECURITY_ROOTPLUG
 	  
 	  If you are unsure how to answer this question, answer N.
 
+config PROPOLICE
+       bool 'GCC ProPolice SSP build support'
+       help
+         This enables kernel building with stack-smashing protection
+         via the -fstack-protector GCC flag, if you have GCC build with
+	 propolice.
+
+	 See <http://www.research.ibm.com/trl/projects/security/ssp/> for
+	 more information about this compiler-extension.
+
+	 If you are unsure how to answer this question, answer N.
+
 source security/selinux/Kconfig
 
 endmenu
--- linux-2.6.9/Makefile.orig	2005-01-13 16:38:39.479192744 +0100
+++ linux-2.6.9/Makefile	2005-01-13 16:40:45.139089536 +0100
@@ -490,6 +490,10 @@ ifndef CONFIG_FRAME_POINTER
 CFLAGS		+= -fomit-frame-pointer
 endif
 
+ifdef CONFIG_PROPOLICE
+CFLAGS		+= -fstack-protector
+endif
+
 ifdef CONFIG_DEBUG_INFO
 CFLAGS		+= -g
 endif
--- linux-2.6.9/lib/propolice.c.orig	2005-01-13 17:08:49.920963760 +0100
+++ linux-2.6.9/lib/propolice.c	2005-01-14 11:23:14.786142384 +0100
@@ -0,0 +1,22 @@
+/*
+ * Copyright 2005, Han Boetes <han@boetes.org>
+ *
+ * This code adds support for the propolice stacksmashing
+ * extension for gcc.
+ * http://www.research.ibm.com/trl/projects/security/ssp/
+ *
+ * This source code is licensed under the GNU General Public
+ * License, Version 2. See the file COPYING for more details.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+unsigned char __guard[sizeof(int)] = {'\0', '\0', '\n', (unsigned char) -1};
+EXPORT_SYMBOL(__guard);
+
+void __stack_smash_handler(unsigned int damaged, const char *func)
+{
+    panic("propolice detects %x at function %s.\n", damaged, func);
+}
+EXPORT_SYMBOL(__stack_smash_handler);


# Han

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-14  7:06     ` Ulrich Drepper
@ 2005-01-14 14:08       ` Nix
  0 siblings, 0 replies; 23+ messages in thread
From: Nix @ 2005-01-14 14:08 UTC (permalink / raw)
  To: Ulrich Drepper; +Cc: linux-kernel

On 14 Jan 2005, Ulrich Drepper yowled:
> Finally, the gcc patch is not going to work as is on architectures
> like IA-64 which do not have the kind of adressing modes which are
> needed for the code to work.

Nor does it work on SPARC-like processors (it tries, but the resulting
programs dump core).

> To fully understand the problem, you need to understand compiler
> design, and especially RTL.  The latter by itself is another problem:
> getting the code work in gcc 4 is at least challenging due the SSA.

Probably a rewrite (using trees instead of RTL) would be easier;
preferably by someone who actually documents what the patch is doing and
submits it in a fashion acceptable to the GCC people; i.e. not in a
single huge nearly-unexplained lump followed by all queries going
unanswered.

Working directly on trees would have the advantage that it would work on
every platform GCC supports straight away (or at least it would have a
higher chance of doing so).

-- 
`Blish is clearly in love with language. Unfortunately,
 language dislikes him intensely.' --- Russ Allbery

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-14 10:30       ` Han Boetes
@ 2005-01-15  2:25         ` Mitchell Blank Jr
  2005-01-15  8:10           ` Han Boetes
  0 siblings, 1 reply; 23+ messages in thread
From: Mitchell Blank Jr @ 2005-01-15  2:25 UTC (permalink / raw)
  To: Han Boetes; +Cc: linux-kernel

Last nitpicks, I swear:

Han Boetes wrote:
> +    panic("propolice detects %x at function %s.\n", damaged, func);

You don't need the "\n" at the end - panic() already includes a newline
Most panic() messages don't seem to have punctuation either so you can
probably lose the "." too.

Also it's nice to use "0x%X" to print hex numbers so they don't ever
get misinterpreted as decimals.

-Mitch

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: propolice support for linux
  2005-01-15  2:25         ` Mitchell Blank Jr
@ 2005-01-15  8:10           ` Han Boetes
  0 siblings, 0 replies; 23+ messages in thread
From: Han Boetes @ 2005-01-15  8:10 UTC (permalink / raw)
  To: linux-kernel

Mitchell Blank Jr wrote:
> Last nitpicks, I swear:

*g* Would we have created the perfect code ;-)

> Han Boetes wrote:
> > +    panic("propolice detects %x at function %s.\n", damaged, func);
>
> You don't need the "\n" at the end - panic() already includes a
> newline Most panic() messages don't seem to have punctuation
> either so you can probably lose the "." too.
>
> Also it's nice to use "0x%X" to print hex numbers so they don't
> ever get misinterpreted as decimals.

Ok, here goes the final version, dear mailinglist-archive-delver.
I hope you found this version and didn't stop looking at the first
hit you found on google. Do read the whole thread, there are some
very knowledgeable people giving their opinion.


--- linux-2.6.9/lib/Makefile.orig	2005-01-13 16:47:58.564198904 +0100
+++ linux-2.6.9/lib/Makefile	2005-01-13 17:06:29.124368096 +0100
@@ -23,6 +23,8 @@ obj-$(CONFIG_GENERIC_IOMAP) += iomap.o
 obj-$(CONFIG_ZLIB_INFLATE) += zlib_inflate/
 obj-$(CONFIG_ZLIB_DEFLATE) += zlib_deflate/
 
+obj-$(CONFIG_PROPOLICE) += propolice.o
+
 hostprogs-y	:= gen_crc32table
 clean-files	:= crc32table.h
 
--- linux-2.6.9/security/Kconfig.orig	2004-10-18 23:54:39.000000000 +0200
+++ linux-2.6.9/security/Kconfig	2005-01-13 16:57:23.130371800 +0100
@@ -44,6 +44,18 @@ config SECURITY_ROOTPLUG
 	  
 	  If you are unsure how to answer this question, answer N.
 
+config PROPOLICE
+       bool 'GCC ProPolice SSP build support'
+       help
+         This enables kernel building with stack-smashing protection
+         via the -fstack-protector GCC flag, if you have GCC build with
+	 propolice.
+
+	 See <http://www.research.ibm.com/trl/projects/security/ssp/> for
+	 more information about this compiler-extension.
+
+	 If you are unsure how to answer this question, answer N.
+
 source security/selinux/Kconfig
 
 endmenu
--- linux-2.6.9/Makefile.orig	2005-01-13 16:38:39.479192744 +0100
+++ linux-2.6.9/Makefile	2005-01-13 16:40:45.139089536 +0100
@@ -490,6 +490,10 @@ ifndef CONFIG_FRAME_POINTER
 CFLAGS		+= -fomit-frame-pointer
 endif
 
+ifdef CONFIG_PROPOLICE
+CFLAGS		+= -fstack-protector
+endif
+
 ifdef CONFIG_DEBUG_INFO
 CFLAGS		+= -g
 endif
--- linux-2.6.9/lib/propolice.c.orig	2005-01-13 17:08:49.920963760 +0100
+++ linux-2.6.9/lib/propolice.c	2005-01-14 11:23:14.786142384 +0100
@@ -0,0 +1,22 @@
+/*
+ * Copyright 2005, Han Boetes <han@boetes.org>
+ *
+ * This code adds support for the propolice stacksmashing
+ * extension for gcc.
+ * http://www.research.ibm.com/trl/projects/security/ssp/
+ *
+ * This source code is licensed under the GNU General Public
+ * License, Version 2. See the file COPYING for more details.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+unsigned char __guard[sizeof(int)] = {'\0', '\0', '\n', (unsigned char) -1};
+EXPORT_SYMBOL(__guard);
+
+void __stack_smash_handler(unsigned int damaged, const char *func)
+{
+    panic("propolice detects 0x%X at function %s", damaged, func);
+}
+EXPORT_SYMBOL(__stack_smash_handler);



# Han

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2005-01-15  8:10 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-01-13 13:46 propolice support for linux Han Boetes
2005-01-13 14:04 ` Christoph Hellwig
2005-01-13 14:53   ` Arjan van de Ven
2005-01-13 16:37   ` Han Boetes
2005-01-13 17:02     ` Arjan van de Ven
2005-01-13 19:32       ` Han Boetes
2005-01-14  7:35         ` Arjan van de Ven
2005-01-13 17:07     ` Bill Davidsen
2005-01-13 17:31       ` Han Boetes
2005-01-13 17:58     ` Zwane Mwaikambo
2005-01-13 18:17       ` Han Boetes
2005-01-14  4:25     ` Mitchell Blank Jr
2005-01-14 10:30       ` Han Boetes
2005-01-15  2:25         ` Mitchell Blank Jr
2005-01-15  8:10           ` Han Boetes
2005-01-13 14:07 ` Arjan van de Ven
2005-01-13 14:15 ` Jakub Jelinek
2005-01-13 19:58 ` Andi Kleen
2005-01-13 21:11 ` Ulrich Drepper
2005-01-13 22:52   ` Han Boetes
2005-01-14  6:25     ` Willy Tarreau
2005-01-14  7:06     ` Ulrich Drepper
2005-01-14 14:08       ` Nix

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).