[PATCH 3/3] Add support for OpenBSD

[PATCH 3/3] Add support for OpenBSD

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 253 bytes --]


Hi!

This is the third and last patch.

This patch makes the xen kernel buildable on OpenBSD by adding support for 
ProPolice. ProPolice has been added to standard GCC in version 4.1.x under
the name Stack Smashing Protection (SSP).

Cheers
Christoph


[-- Attachment #2: openbsd_propolice.diff --]
[-- Type: text/x-diff, Size: 2702 bytes --]

diff -r bd207697f0c7 xen/arch/x86/boot/mkelf32.c
--- a/xen/arch/x86/boot/mkelf32.c	Wed Oct 18 13:43:35 2006 +0100
+++ b/xen/arch/x86/boot/mkelf32.c	Tue Oct 17 16:39:13 2006 +0200
@@ -90,9 +90,15 @@ static Elf32_Shdr out_shdr[] = {
     }
 };
 
+#ifndef swap16
 #define swap16(_v) ((((u16)(_v)>>8)&0xff)|(((u16)(_v)&0xff)<<8))
+#endif
+#ifndef swap32
 #define swap32(_v) (((u32)swap16((u16)(_v))<<16)|(u32)swap16((u32)((_v)>>16)))
+#endif
+#ifndef swap64
 #define swap64(_v) (((u64)swap32((u32)(_v))<<32)|(u64)swap32((u32)((_v)>>32)))
+#endif
 
 static int big_endian;
 
diff -r bd207697f0c7 xen/arch/x86/setup.c
--- a/xen/arch/x86/setup.c	Wed Oct 18 13:43:35 2006 +0100
+++ b/xen/arch/x86/setup.c	Tue Oct 17 16:39:13 2006 +0200
@@ -16,6 +16,7 @@
 #include <xen/percpu.h>
 #include <xen/hypercall.h>
 #include <xen/keyhandler.h>
+#include <xen/guard.h>
 #include <public/version.h>
 #include <asm/bitops.h>
 #include <asm/smp.h>
@@ -625,6 +626,8 @@ void __init __start_xen(multiboot_info_t
                         cmdline) != 0)
         panic("Could not set up DOM0 guest OS\n");
 
+    init_guard();
+
     /* Scrub RAM that is still free and so may go to an unprivileged domain. */
     scrub_heap_pages();
 
diff -r bd207697f0c7 xen/common/Makefile
--- a/xen/common/Makefile	Wed Oct 18 13:43:35 2006 +0100
+++ b/xen/common/Makefile	Tue Oct 17 16:39:13 2006 +0200
@@ -5,6 +5,7 @@ obj-y += elf.o
 obj-y += elf.o
 obj-y += event_channel.o
 obj-y += grant_table.o
+obj-y += guard.o
 obj-y += kernel.o
 obj-y += keyhandler.o
 obj-y += lib.o
diff -r bd207697f0c7 xen/common/guard.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/xen/common/guard.c	Tue Oct 17 16:39:13 2006 +0200
@@ -0,0 +1,34 @@
+
+#include <xen/lib.h>
+
+long __guard[8];
+
+void __stack_smash_handler(char [], int __attribute__((unused)));
+
+void
+__stack_smash_handler(char func[], int damaged)
+{
+        panic("smashed stack in %s", func);
+}
+
+
+void init_guard(void)
+{
+	volatile long newguard[8];
+	int i;
+
+	/* XXX newguard is intended to get filled with random values.
+	 * But there's nothing to use, so this is just something
+	 * out of my head :) */
+	newguard[0] = 103958;
+	newguard[1] = 3505;
+	newguard[2] = 75601;
+	newguard[3] = 35703;
+	newguard[4] = 94;
+	newguard[5] = 721;
+	newguard[6] = 3094;
+	newguard[7] = 217;
+
+	for (i = sizeof(__guard) / sizeof(__guard[0]) - 1; i; i--)
+		__guard[i] = newguard[i];
+}
diff -r bd207697f0c7 xen/include/xen/guard.h
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/xen/include/xen/guard.h	Tue Oct 17 16:39:13 2006 +0200
@@ -0,0 +1,3 @@
+
+
+void init_guard(void);
+

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 3/3] Add support for OpenBSD

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 1866 bytes --]

On Wednesday 18 October 2006 17:51, Keir Fraser wrote:
> On 17/10/06 15:44, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> > This is the third and last patch.
> >
> > This patch makes the xen kernel buildable on OpenBSD by adding support
> > for ProPolice. ProPolice has been added to standard GCC in version 4.1.x
> > under the name Stack Smashing Protection (SSP).
>
> Gcc 4.1.x works fine for me already (under Linux at least). We specifically
> disable stack protection in xen/arch/x86/Rules.mk.

The stack protection is not just to improve security. With a stack protection,
it is more likely that you find off-by-one bugs like this:

void foo(void)
{
        char array[8];
        int i;

        for (i = 0; i <= 8; i++) {
             array[i] = 0;
        }
        .....
}


The propolice patch also contained a snippet, which necessary to make
the Xen kernel build independ if SSP is disabled or not.

I extracted this snippet into a separate patch to fix this build error:

gcc -Wall -Werror -Wstrict-prototypes -O2 -fomit-frame-pointer  -o 
boot/mkelf32 boot/mkelf32.c
boot/mkelf32.c:93:1: "swap16" redefined
In file included from /usr/include/machine/endian.h:68,
                 from /usr/include/sys/types.h:45,
                 from /usr/include/stdio.h:45,
                 from boot/mkelf32.c:11:
/usr/include/sys/endian.h:156:1: this is the location of the previous 
definition
boot/mkelf32.c:94:1: "swap32" redefined
/usr/include/sys/endian.h:157:1: this is the location of the previous 
definition
boot/mkelf32.c:95:1: "swap64" redefined
/usr/include/sys/endian.h:158:1: this is the location of the previous 
definition
gmake[2]: *** [boot/mkelf32] Error 1

This mail has two patches attached:
openbsd_buildfix.diff  - the absolute necessary patch to make the kernel build 
on OpenBSD
openbsd_propolice.diff - it adds support for SSP



[-- Attachment #2: openbsd_buildfix.diff --]
[-- Type: text/x-diff, Size: 589 bytes --]

diff -r 20522afb2615 xen/arch/x86/boot/mkelf32.c
--- a/xen/arch/x86/boot/mkelf32.c	Wed Oct 18 19:23:32 2006 +0100
+++ b/xen/arch/x86/boot/mkelf32.c	Wed Oct 18 09:03:19 2006 +0200
@@ -90,9 +90,15 @@ static Elf32_Shdr out_shdr[] = {
     }
 };
 
+#ifndef swap16
 #define swap16(_v) ((((u16)(_v)>>8)&0xff)|(((u16)(_v)&0xff)<<8))
+#endif
+#ifndef swap32
 #define swap32(_v) (((u32)swap16((u16)(_v))<<16)|(u32)swap16((u32)((_v)>>16)))
+#endif
+#ifndef swap64
 #define swap64(_v) (((u64)swap32((u32)(_v))<<32)|(u64)swap32((u32)((_v)>>32)))
+#endif
 
 static int big_endian;
 

[-- Attachment #3: openbsd_propolice.diff --]
[-- Type: text/x-diff, Size: 2110 bytes --]

diff -r 20522afb2615 xen/arch/x86/setup.c
--- a/xen/arch/x86/setup.c	Wed Oct 18 19:23:32 2006 +0100
+++ b/xen/arch/x86/setup.c	Wed Oct 18 09:03:33 2006 +0200
@@ -16,6 +16,7 @@
 #include <xen/percpu.h>
 #include <xen/hypercall.h>
 #include <xen/keyhandler.h>
+#include <xen/guard.h>
 #include <public/version.h>
 #include <asm/bitops.h>
 #include <asm/smp.h>
@@ -668,6 +669,8 @@ void __init __start_xen(multiboot_info_t
                         cmdline) != 0)
         panic("Could not set up DOM0 guest OS\n");
 
+    init_guard();
+
     /* Scrub RAM that is still free and so may go to an unprivileged domain. */
     scrub_heap_pages();
 
diff -r 20522afb2615 xen/common/Makefile
--- a/xen/common/Makefile	Wed Oct 18 19:23:32 2006 +0100
+++ b/xen/common/Makefile	Wed Oct 18 09:03:33 2006 +0200
@@ -5,6 +5,7 @@ obj-y += elf.o
 obj-y += elf.o
 obj-y += event_channel.o
 obj-y += grant_table.o
+obj-y += guard.o
 obj-y += kernel.o
 obj-y += keyhandler.o
 obj-y += lib.o
diff -r 20522afb2615 xen/common/guard.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/xen/common/guard.c	Wed Oct 18 09:03:33 2006 +0200
@@ -0,0 +1,34 @@
+
+#include <xen/lib.h>
+
+long __guard[8];
+
+void __stack_smash_handler(char [], int __attribute__((unused)));
+
+void
+__stack_smash_handler(char func[], int damaged)
+{
+        panic("smashed stack in %s", func);
+}
+
+
+void init_guard(void)
+{
+	volatile long newguard[8];
+	int i;
+
+	/* XXX newguard is intended to get filled with random values.
+	 * But there's nothing to use, so this is just something
+	 * out of my head :) */
+	newguard[0] = 103958;
+	newguard[1] = 3505;
+	newguard[2] = 75601;
+	newguard[3] = 35703;
+	newguard[4] = 94;
+	newguard[5] = 721;
+	newguard[6] = 3094;
+	newguard[7] = 217;
+
+	for (i = sizeof(__guard) / sizeof(__guard[0]) - 1; i; i--)
+		__guard[i] = newguard[i];
+}
diff -r 20522afb2615 xen/include/xen/guard.h
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/xen/include/xen/guard.h	Wed Oct 18 09:03:33 2006 +0200
@@ -0,0 +1,3 @@
+
+
+void init_guard(void);

[-- Attachment #4: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 3/3] Add support for OpenBSD

[Christoph Egger]

On Thursday 19 October 2006 09:51, Keir Fraser wrote:
> On 18/10/06 8:08 am, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> > /usr/include/sys/endian.h:156:1: this is the location of the previous
> > definition
> > boot/mkelf32.c:94:1: "swap32" redefined
> > /usr/include/sys/endian.h:157:1: this is the location of the previous
> > definition
> > boot/mkelf32.c:95:1: "swap64" redefined
> > /usr/include/sys/endian.h:158:1: this is the location of the previous
> > definition
>
> Is there a standard definition for these functions (i.e., can we be *sure*
> the original definitions have the same semantics as our own)? If not, which
> I think is the case, we're better off #undef'ing the old definitions.

This is the snippet of /usr/include/endian.h:

-----------------------------------------------------------------------------------
#ifdef __GNUC__

#define __swap16gen(x) __statement({                                    \
        __uint16_t __swap16gen_x = (x);                                 \
                                                                        \
        (__uint16_t)((__swap16gen_x & 0xff) << 8 |                      \
            (__swap16gen_x & 0xff00) >> 8);                             \
})

#define __swap32gen(x) __statement({                                    \
        __uint32_t __swap32gen_x = (x);                                 \
                                                                        \
        (__uint32_t)((__swap32gen_x & 0xff) << 24 |                     \
            (__swap32gen_x & 0xff00) << 8 |                             \
            (__swap32gen_x & 0xff0000) >> 8 |                           \
            (__swap32gen_x & 0xff000000) >> 24);                        \
})

#define __swap64gen(x) __statement({                                    \
        __uint64_t __swap64gen_x = (x);                                 \
                                                                        \
        (__uint64_t)((__swap64gen_x & 0xff) << 56 |                     \
            (__swap64gen_x & 0xff00ULL) << 40 |                         \
            (__swap64gen_x & 0xff0000ULL) << 24 |                       \
            (__swap64gen_x & 0xff000000ULL) << 8 |                      \
            (__swap64gen_x & 0xff00000000ULL) >> 8 |                    \
            (__swap64gen_x & 0xff0000000000ULL) >> 24 |                 \
            (__swap64gen_x & 0xff000000000000ULL) >> 40 |               \
            (__swap64gen_x & 0xff00000000000000ULL) >> 56);             \
})

#else /* __GNUC__ */

/* Note that these macros evaluate their arguments several times.  */
#define __swap16gen(x)                                                  \
    (__uint16_t)(((__uint16_t)(x) & 0xffU) << 8 | ((__uint16_t)(x) & 0xff00U) 
>> 8)

#define __swap32gen(x)                                                  \
    (__uint32_t)(((__uint32_t)(x) & 0xff) << 24 |                       \
    ((__uint32_t)(x) & 0xff00) << 8 | ((__uint32_t)(x) & 0xff0000) >> 8 |\
    ((__uint32_t)(x) & 0xff000000) >> 24)

#define __swap64gen(x)                                                  \
        (__uint64_t)((((__uint64_t)(x) & 0xff) << 56) |                 \
            ((__uint64_t)(x) & 0xff00ULL) << 40 |                       \
            ((__uint64_t)(x) & 0xff0000ULL) << 24 |                     \
            ((__uint64_t)(x) & 0xff000000ULL) << 8 |                    \
            ((__uint64_t)(x) & 0xff00000000ULL) >> 8 |                  \
            ((__uint64_t)(x) & 0xff0000000000ULL) >> 24 |               \
            ((__uint64_t)(x) & 0xff000000000000ULL) >> 40 |             \
            ((__uint64_t)(x) & 0xff00000000000000ULL) >> 56)

#endif /* __GNUC__ */
-----------------------------------------------------------------------------------

Re: [PATCH 3/3] Add support for OpenBSD

[Keir Fraser]

On 17/10/06 15:44, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> This is the third and last patch.
> 
> This patch makes the xen kernel buildable on OpenBSD by adding support for
> ProPolice. ProPolice has been added to standard GCC in version 4.1.x under
> the name Stack Smashing Protection (SSP).

Gcc 4.1.x works fine for me already (under Linux at least). We specifically
disable stack protection in xen/arch/x86/Rules.mk.

 -- Keir

Re: [PATCH 3/3] Add support for OpenBSD

[Keir Fraser]

On 18/10/06 8:08 am, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> /usr/include/sys/endian.h:156:1: this is the location of the previous
> definition
> boot/mkelf32.c:94:1: "swap32" redefined
> /usr/include/sys/endian.h:157:1: this is the location of the previous
> definition
> boot/mkelf32.c:95:1: "swap64" redefined
> /usr/include/sys/endian.h:158:1: this is the location of the previous
> definition

Is there a standard definition for these functions (i.e., can we be *sure*
the original definitions have the same semantics as our own)? If not, which
I think is the case, we're better off #undef'ing the old definitions.

 -- Keir

Re: [PATCH 3/3] Add support for OpenBSD

[Keir Fraser]

On 18/10/06 09:55, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> Is there a standard definition for these functions (i.e., can we be *sure*
>> the original definitions have the same semantics as our own)? If not, which
>> I think is the case, we're better off #undef'ing the old definitions.
> 
> This is the snippet of /usr/include/endian.h:

Well, that doesn't indicate whether the semantics of swap<16,32,etc> is
standardised or not. I'll fix in mkelf32 by undef'ing.

 -- Keir

Re: [PATCH 3/3] Add support for OpenBSD

[Christoph Egger]

On Thursday 19 October 2006 13:23, Keir Fraser wrote:
> On 18/10/06 09:55, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> >> Is there a standard definition for these functions (i.e., can we be
> >> *sure* the original definitions have the same semantics as our own)? If
> >> not, which I think is the case, we're better off #undef'ing the old
> >> definitions.
> >
> > This is the snippet of /usr/include/endian.h:
>
> Well, that doesn't indicate whether the semantics of swap<16,32,etc> is
> standardised or not. I'll fix in mkelf32 by undef'ing.

Thanks. I'm awaiting your commit. :-)