Re: Stack protection via GCC's `-fstack-protector'

[Thomas Schwinge]

Hello!

On Wed, Nov 08, 2006 at 11:36:20PM +0100, I wrote:
> On Wed, Nov 08, 2006 at 11:14:19PM +0100, Samuel Thibault wrote:
> > Thomas Schwinge, le Wed 08 Nov 2006 22:40:54 +0100, a ?crit :
> > > Is it feasible to have the `-fstack-protector' functionality in GNU Mach
> > > and GRUB2 (and how to do that, then) or shall we unconditionally pass
> > > `-fno-stack-protector' if available?
> > 
> > I'd say it shouldn't be very hard to provide a stack_chk_fail function,
> > even if all it does is just panic().
> 
> Having had a look at glibc's implementation and its complexity ;-) that
> might indeed be a reachable goal (assuming that no other surprises show
> up):

There were other surprises (namely a GCC bug), but see below: what
follows there is a proposal for GNU Mach, the one for GNU GRUB2 would
look somewhat similar.


To be discussed is the initialization of `__stack_chk_guard'.  As I
understand it, this is --- in user space programs and the OpenBSD kernel
--- done by using some (pseudo)random values or --- if that's not
possible in user space or in the DragonflyBSD kernel --- by using some
special characters (`nul', `\n', `0x255'), like e.g.:

dragonflybsd/src/sys/libkern/stack_protector.c
#v+
[...]
#if BYTE_ORDER == LITTLE_ENDIAN
int __guard = 0x00000aff;
#else
int __guard = 0xff0a0000;
#endif
[...]
#v-

Other examples:

openbsd/src/sys/kern/init_main.c
#v+
[...]
long    __guard[8];
[...]
        {
                volatile long newguard[8];
                int i;

                arc4random_bytes((long *)newguard, sizeof(newguard));

                for (i = sizeof(__guard)/sizeof(__guard[0]) - 1; i; i--)
                        __guard[i] = newguard[i];
        }
[...]
#v-

user space gcc/libssp/ssp.c (the same is basically done in glibc's
sysdeps/unix/sysv/linux/dl-osinfo.h, sysdeps/generic/dl-osinfo.h)
#v+
[...]
void *__stack_chk_guard = 0;
[...]
static void __attribute__ ((constructor))
__guard_setup (void)
{
  unsigned char *p;
  int fd;

  if (__stack_chk_guard != 0)
    return;

  fd = open ("/dev/urandom", O_RDONLY);
  if (fd != -1)
    {
      ssize_t size = read (fd, &__stack_chk_guard,
                           sizeof (__stack_chk_guard));
      close (fd);
      if (size == sizeof(__stack_chk_guard) && __stack_chk_guard != 0)
        return;
    }

  /* If a random generator can't be used, the protector switches the guard
     to the "terminator canary".  */
  p = (unsigned char *) &__stack_chk_guard;
  p[sizeof(__stack_chk_guard)-1] = 255;
  p[sizeof(__stack_chk_guard)-2] = '\n';
  p[0] = 0;
}
[...]
#v-

What do we want in GNU Mach?


The patch follows:

Index: configure.ac
===================================================================
RCS file: /cvsroot/hurd/gnumach/Attic/configure.ac,v
retrieving revision 1.1.2.6
diff -u -p -r1.1.2.6 configure.ac
--- configure.ac        5 Nov 2006 20:50:25 -0000       1.1.2.6
+++ configure.ac        13 Nov 2006 17:54:53 -0000
@@ -83,6 +83,67 @@ dnl See below why we need to patch stuff
 AC_CHECK_PROG([PATCH], [patch], [patch], [patch-not-found])
 
 #
+# Compiler features.
+#
+
+# Smashing stack protector.
+
+AC_ARG_ENABLE([smashing-stack-protector],
+  AS_HELP_STRING([--disable-smashing-stack-protector],
+    [disable the smashing stack protector]))
+[# Default to using it, if possible.
+enable_smashing_stack_protector=${enable_smashing_stack_protector-auto}
+ssp_possible=no
+# There was a bug in certain versions of GCC that made it emit incorrect code
+# when used in a non-glibc environment.
+ssp_usable=no
+if [ x"$enable_smashing_stack_protector" != xno ]; then]
+  AC_MSG_CHECKING([whether the compiler accepts `-fstack-protector' and the \
+resulting code is suitable in a kernel environment])
+  AC_LANG_CONFTEST([[void foo (void) { volatile char a[8]; a[3]; }]])
+  [# `$CC -c -o ...' might not be portable.  But, oh, well...
+  # Is calling `ac_compile' like this correct, after all?
+  if eval "$ac_compile -S -fstack-protector -o conftest.s"; then
+    ssp_possible=yes
+    if grep -q '%gs' conftest.s; then]
+      AC_MSG_RESULT([accepted, but not usable])
+    [else]
+      AC_MSG_RESULT([yes])
+      [ssp_usable=yes
+    fi
+    # Should we clear up other files as well, having called `AC_LANG_CONFTEST'?
+    rm -f conftest.s
+  else]
+    AC_MSG_RESULT([not accepted])
+  [fi
+fi
+enable_smashing_stack_protector=\
+$enable_smashing_stack_protector-$ssp_possible-$ssp_usable
+case $enable_smashing_stack_protector in
+  no-*) :;;
+  yes-no-*)] AC_MSG_ERROR([cannot use the smashing stack protector as it's \
+not supported by the compiler.])[;;
+  yes-*-no)] AC_MSG_ERROR([cannot use the smashing stack protector as it's \
+not properly supported by the compiler.])[;;
+  *-no-*)] AC_MSG_WARN([won't use the smashing stack protector as it's not \
+supported by the compiler.])[;;
+  *-*-no)] AC_MSG_WARN([won't use the smashing stack protector as it's not \
+properly supported by the compiler.])[;;
+  *-yes-yes) CFLAGS=$CFLAGS\ -fstack-protector
+            enable_smashing_stack_protector=yes
+            # This definition isn't used anywhere, but it's useful for having
+            # all files recompiled if an already-built tree is reconfigured
+            # to switch from using the smashing stack protector to not using
+            # it and vice versa.]
+            AC_DEFINE([USING_SMASHING_STACK_PROTECTOR], [],
+              [Are we using the smashing stack protector?])[;;
+  *)] AC_MSG_ERROR([please report to <$PACKAGE_BUGREPORT> that `configure.ac' \
+is buggy w.r.t. smashing stack protector detection.])[;;
+esac]
+AM_CONDITIONAL([enable_smashing_stack_protector],
+  [[[ x"$enable_smashing_stack_protector" = xyes ]]])
+
+#
 # configure fragments.
 #
 
Index: Makefrag.am
===================================================================
RCS file: /cvsroot/hurd/gnumach/Attic/Makefrag.am,v
retrieving revision 1.1.2.6
diff -u -p -r1.1.2.6 Makefrag.am
--- Makefrag.am 8 Nov 2006 18:55:24 -0000       1.1.2.6
+++ Makefrag.am 13 Nov 2006 17:54:53 -0000
@@ -111,6 +111,11 @@ libkernel_a_SOURCES += \
        util/putchar.c \
        util/puts.c
 
+if enable_smashing_stack_protector
+libkernel_a_SOURCES += \
+       util/stack_chk_fail.c
+endif
+
 # Virtual memory implementation.
 libkernel_a_SOURCES += \
        vm/vm_debug.c \
Index: util/stack_chk_fail.c
===================================================================
RCS file: util/stack_chk_fail.c
diff -N util/stack_chk_fail.c
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ util/stack_chk_fail.c       13 Nov 2006 17:54:53 -0000
@@ -0,0 +1,33 @@
+/*
+ * Support for the smashing stack protector.
+ *
+ * Copyright (C) 2006 Free Software Foundation, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <kern/debug.h>
+
+unsigned int __stack_chk_guard = 12345;
+
+void
+__attribute__ ((noreturn))
+__stack_chk_fail (void)
+{
+  panic ("*** stack smashing detected ***\n"
+        "TODO: Explain.\n"
+        "TODO: Please do whatever and report to <" PACKAGE_BUGREPORT
+        "> or don't.");
+}
Index: doc/mach.texi
===================================================================
RCS file: /cvsroot/hurd/gnumach/doc/mach.texi,v
retrieving revision 1.4.2.5
diff -u -p -r1.4.2.5 mach.texi
--- doc/mach.texi       23 Sep 2006 20:25:40 -0000      1.4.2.5
+++ doc/mach.texi       13 Nov 2006 17:54:56 -0000
@@ -547,6 +547,11 @@ Sets the prefix to PREFIX.  The default 
 is the correct value for the GNU system.  The prefix is prepended to all
 file names at installation time.
 
address@hidden --disable-smashing-stack-protector
+Specifying this, the smashing stack protector is disabled.  By
+default it is enabled if the compiler has proper support for it.
+TODO: Explain what this does.
+
 @item --enable-kdb
 Enables the in-kernel debugger.  This is only useful if you actually
 anticipate debugging the kernel.  It is not enabled by default because


What follows now is the snippet I used for testing this...

Index: i386/i386at/i386at_ds_routines.c
===================================================================
RCS file: /cvsroot/hurd/gnumach/i386/i386at/Attic/i386at_ds_routines.c,v
retrieving revision 1.4.2.5
diff -u -p -r1.4.2.5 i386at_ds_routines.c
--- i386/i386at/i386at_ds_routines.c    11 Nov 2006 00:54:05 -0000      1.4.2.5
+++ i386/i386at/i386at_ds_routines.c    13 Nov 2006 17:54:56 -0000
@@ -74,6 +74,16 @@ ds_device_open (ipc_port_t open_port, ip
   int i;
   io_return_t err;
 
+  void crash_kernel_now (void)
+    {
+      volatile char a[8];
+      printf ("Preparing to crash the kernel...\n");
+      a[8] = 42;
+    }
+
+  if (name && name[0] == 'c' && name[1] == 0)
+    crash_kernel_now ();
+
   /* Open must be called on the master device port.  */
   if (open_port != master_device_port)
     return D_INVALID_OPERATION;

... that one and then a doughty ``sudo devprobe c'' and I found myself in
the kernel debugger and a ``*** stack smashing detected ***[...]''
message on the screen, as expected.


Regards,
 Thomas

signature.asc
Description: Digital signature