2005-01-13 13:47:40

by Han

[permalink] [raw]
Subject: propolice support for linux

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


2005-01-13 14:04:51

by Christoph Hellwig

[permalink] [raw]
Subject: Re: propolice support for linux

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


2005-01-13 14:07:34

by Arjan van de Ven

[permalink] [raw]
Subject: Re: propolice support for linux

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


2005-01-13 14:15:43

by Jakub Jelinek

[permalink] [raw]
Subject: Re: propolice support for linux

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

2005-01-13 14:54:31

by Arjan van de Ven

[permalink] [raw]
Subject: Re: propolice support for linux


> EXPORT_SYMBOL_NOVERS(__guard);

_NOVERS is dead btw

2005-01-13 16:37:44

by Han

[permalink] [raw]
Subject: Re: propolice support for linux


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 <[email protected]>
+ *
+ * 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

2005-01-13 17:09:15

by Bill Davidsen

[permalink] [raw]
Subject: Re: propolice support for linux

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 ([email protected])
"The secret to procrastination is to put things off until the
last possible moment - but no longer" -me

2005-01-13 17:34:33

by Han

[permalink] [raw]
Subject: Re: propolice support for linux

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

2005-01-13 18:00:30

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: propolice support for linux

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.

2005-01-13 18:25:05

by Han

[permalink] [raw]
Subject: Re: propolice support for linux

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 <[email protected]>
+ *
+ * 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

2005-01-13 17:06:08

by Arjan van de Ven

[permalink] [raw]
Subject: Re: propolice support for linux

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)

2005-01-13 20:07:08

by Andi Kleen

[permalink] [raw]
Subject: Re: propolice support for linux

Han Boetes <[email protected]> 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

2005-01-13 21:18:53

by Ulrich Drepper

[permalink] [raw]
Subject: Re: propolice support for linux

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.

2005-01-13 22:57:19

by Han

[permalink] [raw]
Subject: Re: propolice support for linux

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

2005-01-13 19:37:21

by Han

[permalink] [raw]
Subject: Re: propolice support for linux

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

2005-01-14 04:19:14

by Mitchell Blank Jr

[permalink] [raw]
Subject: Re: propolice support for linux

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

2005-01-14 06:35:50

by Willy Tarreau

[permalink] [raw]
Subject: Re: propolice support for linux

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

2005-01-14 07:06:12

by Ulrich Drepper

[permalink] [raw]
Subject: Re: propolice support for linux

On Thu, 13 Jan 2005 23:52:22 +0100, Han Boetes <[email protected]> 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.

2005-01-14 07:35:37

by Arjan van de Ven

[permalink] [raw]
Subject: Re: propolice support for linux


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


2005-01-14 10:33:40

by Han

[permalink] [raw]
Subject: Re: propolice support for linux

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 <[email protected]>
+ *
+ * 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

2005-01-14 14:09:04

by Nix

[permalink] [raw]
Subject: Re: propolice support for linux

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

2005-01-15 02:19:00

by Mitchell Blank Jr

[permalink] [raw]
Subject: Re: propolice support for linux

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

2005-01-15 08:10:43

by Han

[permalink] [raw]
Subject: Re: propolice support for linux

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 <[email protected]>
+ *
+ * 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