[<prev] [next>] [day] [month] [year] [list]
Message-ID: <390CE752059EB848A71F4F676EBAB76D3AC298BC@ORSMSX114.amr.corp.intel.com>
Date: Fri, 4 Nov 2016 20:35:40 +0000
From: "LeMay, Michael" <michael.lemay@...el.com>
To: "musl@...ts.openwall.com" <musl@...ts.openwall.com>
Subject: [RFC PATCH v3] support separate stack segment
This patch adds support for the separate stack segment feature that is
enabled by the LLVM Clang -mseparate-stack-seg flag. This feature
hardens SafeStack by defining a limit for DS and ES that is below all
of the safe stacks. Only memory operands that need to refer to the
safe stacks are directed to the SS segment. Thus, even if a pointer
to a safe stack is used by some other memory operand, the segmentation
feature of the CPU will block that access.
The i386 __clone implementation is written in assembly
language, so the compiler is unable to automatically add a stack
segment override prefix to an instruction in that routine that
accesses a safe stack. This patch adds that prefix to the source
code.
The Linux vDSO code may be incompatible with programs that use a
separate stack segment. This patch prevents the vDSO from being
invoked when that feature is enabled.
The developer should add something like
"-I/usr/include/x86_64-linux-gnu/asm" to CPPFLAGS during configuration
so that ldt.h can be included in separate_stack_seg.c.
Signed-off-by: Michael LeMay <michael.lemay@...el.com>
---
Makefile | 31 ++++++-
arch/i386/syscall_arch.h | 9 ++
configure | 10 ++
src/env/__libc_start_main.c | 2 +
src/internal/i386/separate_stack_seg.c | 163 +++++++++++++++++++++++++++++++++
src/internal/safe_stack.c | 18 +++-
src/internal/separate_stack_seg.c | 13 +++
src/thread/i386/clone.s | 3 +-
8 files changed, 240 insertions(+), 9 deletions(-)
create mode 100644 src/internal/i386/separate_stack_seg.c
create mode 100644 src/internal/separate_stack_seg.c
diff --git a/Makefile b/Makefile
index 1eb7dca..a8126ae 100644
--- a/Makefile
+++ b/Makefile
@@ -48,8 +48,8 @@ CFLAGS_C99FSE = -std=c99 -ffreestanding -nostdinc
CFLAGS_ALL = $(CFLAGS_C99FSE)
CFLAGS_ALL += -D_XOPEN_SOURCE=700 -I$(srcdir)/arch/$(ARCH) -I$(srcdir)/arch/generic -Iobj/src/internal -I$(srcdir)/src/internal -Iobj/include -I$(srcdir)/include
CFLAGS_ALL += $(CPPFLAGS) $(CFLAGS_AUTO)
-# This flag is selectively re-added for certain files below.
-CFLAGS_ALL += $(filter-out -fsanitize=safe-stack,$(CFLAGS))
+# These flags are selectively re-added for certain files below.
+CFLAGS_ALL += $(filter-out -fsanitize=safe-stack -mseparate-stack-seg,$(CFLAGS))
LDFLAGS_ALL = $(LDFLAGS_AUTO) $(LDFLAGS)
@@ -134,14 +134,13 @@ NOSSP_SRCS = $(wildcard crt/*.c) \
ldso/dlstart.c ldso/dynlink.c
$(NOSSP_SRCS:%.c=obj/%.o) $(NOSSP_SRCS:%.c=obj/%.lo): CFLAGS_ALL += $(CFLAGS_NOSSP)
+SEP_STACK_SEG_OBJS = $(filter-out $(CRT_OBJS) obj/ldso/dlstart.o,$(ALL_OBJS))
# The safestack attribute will be selectively forced within these files below.
SAFE_STACK_OBJS = $(filter-out \
- $(CRT_OBJS) \
- obj/ldso/dlstart.o \
$(addprefix obj/src/env/__init_tls.,o lo) \
$(addprefix obj/src/env/__libc_start_main.,o lo) \
$(addprefix obj/src/internal/safe_stack.,o lo), \
- $(ALL_OBJS))
+ $(SEP_STACK_SEG_OBJS))
ifeq ($(SAFE_STACK),yes)
@@ -181,6 +180,28 @@ $(SAFE_STACK_OBJS) $(SAFE_STACK_OBJS:%.o=%.lo): CFLAGS_ALL += -fsanitize=safe-st
endif
+ifeq ($(ARCH)$(SUBARCH),i386ss)
+
+CFLAGS_ALL += -DSEP_STACK_SEG=1
+
+$(SEP_STACK_SEG_OBJS) $(SEP_STACK_SEG_OBJS:%.o=%.lo): CFLAGS_ALL += -mseparate-stack-seg
+
+# The following function is run prior to segment restrictions being activated.
+# It contains code that compiles to an instruction that may access either a
+# stack allocation or a non-stack allocation depending on a condition.
+# The X86FixupSeparateStack pass in LLVM does not support such code, since
+# simply directing the affected memory operand to either DS or SS could result
+# in an invalid memory access if DS and SS have different base addresses.
+# Specifying this attribute causes the X86FixupSeparateStack pass to not
+# process this function. An alternative approach could have been to direct such
+# memory operands to SS if DS and SS have the same base address, which is the
+# way that musl configures the segments. However, such compiler support has not
+# been implemented. Regardless, adding segment override prefixes to functions
+# that only run with a flat memory model is superfluous.
+obj/ldso/dynlink.lo: CFLAGS_ALL += -mllvm -sep-stk-seg-flat-mem-func=__dls3
+
+endif
+
$(CRT_OBJS): CFLAGS_ALL += -DCRT
$(LOBJS) $(LDSO_OBJS): CFLAGS_ALL += -fPIC
diff --git a/arch/i386/syscall_arch.h b/arch/i386/syscall_arch.h
index 4c9d874..4d7c3c2 100644
--- a/arch/i386/syscall_arch.h
+++ b/arch/i386/syscall_arch.h
@@ -52,8 +52,17 @@ static inline long __syscall6(long n, long a1, long a2, long a3, long a4, long a
return __ret;
}
+#if !SEP_STACK_SEG
+/* The vDSO may not be compiled with support for a separate stack segment.
+ * Avoid invoking the vDSO when this feature is enabled, since it may try to
+ * access the stack using memory operands with base registers other than EBP or
+ * ESP without also using a stack segment override prefix. A special compiler
+ * pass needs to be used to add such prefixes, and it is unlikely that a pass
+ * of that sort was applied when the vDSO was compiled.
+ */
#define VDSO_USEFUL
#define VDSO_CGT_SYM "__vdso_clock_gettime"
#define VDSO_CGT_VER "LINUX_2.6"
+#endif
#define SYSCALL_USE_SOCKETCALL
diff --git a/configure b/configure
index a70009f..7d1d0b4 100755
--- a/configure
+++ b/configure
@@ -599,6 +599,16 @@ t="$CFLAGS_C99FSE $CPPFLAGS $CFLAGS"
fnmatch '-fsanitize=safe-stack*|*\ -fsanitize=safe-stack*' "$CFLAGS" && SAFE_STACK=yes
+if test "$ARCH" = "i386" ; then
+if fnmatch '-mseparate-stack-seg*|*\ -mseparate-stack-seg*' "$CFLAGS" ; then
+if test "$SAFE_STACK" = "yes" ; then
+SUBARCH=ss
+else
+fail "$0: the separate stack segment feature requires that SafeStack also be enabled"
+fi
+fi
+fi
+
if test "$ARCH" = "x86_64" ; then
trycppif __ILP32__ "$t" && ARCH=x32
fi
diff --git a/src/env/__libc_start_main.c b/src/env/__libc_start_main.c
index dfb4ebb..424cf88 100644
--- a/src/env/__libc_start_main.c
+++ b/src/env/__libc_start_main.c
@@ -19,6 +19,7 @@ weak_alias(dummy1, __init_ssp);
void __preinit_unsafe_stack(void);
void __init_unsafe_stack(void);
+void __sep_stack_seg_init(int argc, char ***argvp, char ***envpp);
#define AUX_CNT 38
@@ -73,6 +74,7 @@ int __libc_start_main(int (*main)(int,char **,char **), int argc, char **argv)
__preinit_unsafe_stack();
__init_libc(envp, argv[0]);
+ __sep_stack_seg_init(argc, &argv, &envp);
__libc_start_init();
/* Pass control to the application */
diff --git a/src/internal/i386/separate_stack_seg.c b/src/internal/i386/separate_stack_seg.c
new file mode 100644
index 0000000..fb796cc
--- /dev/null
+++ b/src/internal/i386/separate_stack_seg.c
@@ -0,0 +1,163 @@
+#define _GNU_SOURCE
+#include "atomic.h"
+#include "libc.h"
+#include "syscall.h"
+#include <malloc.h>
+#include <stdint.h>
+#include <string.h>
+
+#if SEP_STACK_SEG
+
+#include <ldt.h>
+
+char *__strdup(const char *s);
+
+extern uintptr_t __stack_base;
+
+static int modify_ldt(int func, void *ptr, unsigned long bytecount) {
+ return syscall(SYS_modify_ldt, func, ptr, bytecount);
+}
+
+static void update_ldt(size_t len) {
+ struct user_desc stack_desc;
+ stack_desc.entry_number = 0;
+ stack_desc.base_addr = 0;
+ stack_desc.contents = 0; /* data */
+ stack_desc.limit = (int)((len - 1) >> 12);
+ stack_desc.limit_in_pages = 1;
+ stack_desc.read_exec_only = 0;
+ stack_desc.seg_not_present = 0;
+ stack_desc.seg_32bit = 1;
+ stack_desc.useable = 1;
+
+ if (modify_ldt(1, &stack_desc, sizeof(stack_desc)) == -1)
+ a_crash();
+}
+
+static void verify_ldt_empty(void) {
+ uint64_t ldt;
+
+ /* read the current LDT */
+ int ldt_len = modify_ldt(0, &ldt, sizeof(ldt));
+ if (ldt_len != 0)
+ /* LDT not empty */
+ a_crash();
+}
+
+#define SEG_SEL_LDT 4
+#define SEG_SEL_CPL3 3
+/* require restricted segment to occupy the first LDT entry */
+#define SEG_SEL_RESTRICTED (SEG_SEL_LDT | SEG_SEL_CPL3)
+
+__attribute__((__visibility__("hidden")))
+void __restrict_segments(void)
+{
+ uintptr_t limit, stack_base = __stack_base;
+ int data_seg_sel;
+
+ __asm__ __volatile__ ("mov %%ds, %0" : "=r"(data_seg_sel));
+ /* assume that ES is identical to DS */
+
+ if ((data_seg_sel & SEG_SEL_LDT) == SEG_SEL_LDT) {
+ if (data_seg_sel != SEG_SEL_RESTRICTED)
+ a_crash();
+
+ /* Read the current limit from the segment register rather than
+ * relying on __stack_base, since __stack_base is in the
+ * default data segment and could potentially be subject to
+ * memory corruption. */
+ __asm__ __volatile__ ("lsl %1, %0" : "=r"(limit) : "m"(data_seg_sel));
+
+ if (limit < stack_base)
+ return;
+ } else
+ verify_ldt_empty();
+
+ update_ldt(stack_base);
+
+ /* Reload the DS and ES segment registers from the new or updated LDT
+ * entry. */
+ __asm__ __volatile__ (
+ "mov %0, %%ds\n\t"
+ "mov %0, %%es\n\t"
+ ::
+ "r"(SEG_SEL_RESTRICTED)
+ );
+}
+
+extern char **__environ;
+
+/* Programs and much of the libc code expect to be able to access the arguments,
+ * environment, and auxv in DS, but they are initially located on the stack.
+ * This function moves them to the heap. This uses __strdup to copy data from
+ * the stack, so it must run before segment limits are restricted.
+ */
+__attribute__((__visibility__("hidden")))
+void __sep_stack_seg_init(int argc, char ***argvp, char ***envpp)
+{
+ char **argv = *argvp;
+ char **envp = *envpp;
+ char **environ_end = envp;
+ size_t *auxv, *auxv_end;
+ char **new_argv = 0;
+
+ while (*environ_end) environ_end++;
+
+ auxv_end = (size_t *)environ_end + 1;
+ while (*auxv_end) auxv_end++;
+ auxv_end++;
+
+ new_argv = malloc((uintptr_t)auxv_end - (uintptr_t)argvp);
+ if (!new_argv)
+ a_crash();
+
+ *new_argv = (char *)argc;
+ new_argv++;
+
+ *argvp = new_argv;
+
+ for (int i = 0; i < argc; i++)
+ new_argv[i] = __strdup(argv[i]);
+ new_argv += argc;
+ *new_argv = NULL;
+ new_argv++;
+
+ *envpp = __environ = new_argv;
+ while (envp != environ_end) {
+ *new_argv = __strdup(*envp);
+ envp++;
+ new_argv++;
+ }
+ *new_argv = NULL;
+ envp++;
+ new_argv++;
+
+ libc.auxv = (size_t *)new_argv;
+ memcpy(new_argv, envp, (uintptr_t)auxv_end - (uintptr_t)envp);
+
+ __restrict_segments();
+}
+
+uintptr_t __safestack_addr_hint(size_t size)
+{
+ /* Try to allocate the new safe stack just below the lowest existing safe
+ * stack to help avoid a data segment limit that is too low and causes
+ * faults when accessing non-stack data above the limit. */
+
+ return __stack_base - size;
+}
+
+#else
+
+static void dummy(void) {}
+weak_alias(dummy, __restrict_segments);
+static void dummy1(int argc, char ***argvp, char ***envpp) {}
+weak_alias(dummy1, __sep_stack_seg_init);
+
+__attribute__((__visibility__("hidden")))
+uintptr_t __safestack_addr_hint(size_t size)
+{
+ return 0;
+}
+
+#endif /*SEP_STACK_SEG*/
diff --git a/src/internal/safe_stack.c b/src/internal/safe_stack.c
index 86804aa..54df703 100644
--- a/src/internal/safe_stack.c
+++ b/src/internal/safe_stack.c
@@ -8,9 +8,14 @@
static bool unsafe_stack_ptr_inited = false;
+/* base address of safe stack allocated most recently */
+__attribute__((__visibility__("hidden")))
+uintptr_t __stack_base;
+
void *__mmap(void *, size_t, int, int, int, off_t);
int __munmap(void *, size_t);
int __mprotect(void *, size_t, int);
+void __restrict_segments(void);
/* There are no checks for overflows past the end of this stack buffer. It must
* be allocated with adequate space to meet the requirements of all of the code
@@ -21,7 +26,6 @@ static unsigned char preinit_us[4096];
__attribute__((__visibility__("hidden")))
void __init_unsafe_stack(void)
{
- void *stack_base;
size_t stack_size;
pthread_attr_t attr;
struct pthread *self;
@@ -39,7 +43,7 @@ void __init_unsafe_stack(void)
if (pthread_getattr_np(self, &attr) != 0)
a_crash();
- if (pthread_attr_getstack(&attr, &stack_base, &stack_size) != 0)
+ if (pthread_attr_getstack(&attr, (void **)&__stack_base, &stack_size) != 0)
a_crash();
stack_size *= 2;
@@ -77,6 +81,8 @@ void __preinit_unsafe_stack(void)
#define ROUND(x) (((x)+PAGE_SIZE-1)&-PAGE_SIZE)
+uintptr_t __safestack_addr_hint(size_t size);
+
__attribute__((__visibility__("hidden")))
int __safestack_init_thread(struct pthread *restrict new, const pthread_attr_t *restrict attr)
{
@@ -88,7 +94,9 @@ int __safestack_init_thread(struct pthread *restrict new, const pthread_attr_t *
guard = ROUND(DEFAULT_GUARD_SIZE + attr->_a_guardsize);
size = ROUND(new->stack_size + guard);
- map = __mmap(0, size, PROT_NONE, MAP_PRIVATE|MAP_ANON, -1, 0);
+ uintptr_t try_map = __safestack_addr_hint(size);
+
+ map = __mmap((void *)try_map, size, PROT_NONE, MAP_PRIVATE|MAP_ANON, -1, 0);
if (map == MAP_FAILED)
goto fail;
if (__mprotect(map+guard, size-guard, PROT_READ|PROT_WRITE)
@@ -101,6 +109,10 @@ int __safestack_init_thread(struct pthread *restrict new, const pthread_attr_t *
new->safe_stack_size = size;
new->stack = map + size;
+ __stack_base = (uintptr_t)map;
+
+ __restrict_segments();
+
return 0;
fail:
return EAGAIN;
diff --git a/src/internal/separate_stack_seg.c b/src/internal/separate_stack_seg.c
new file mode 100644
index 0000000..fe665e3
--- /dev/null
+++ b/src/internal/separate_stack_seg.c
@@ -0,0 +1,13 @@
+#include "libc.h"
+#include <stdint.h>
+
+static void dummy(void) {}
+weak_alias(dummy, __restrict_segments);
+static void dummy1(int argc, char ***argvp, char ***envpp) {}
+weak_alias(dummy1, __sep_stack_seg_init);
+
+__attribute__((__visibility__("hidden")))
+uintptr_t __safestack_addr_hint(size_t size)
+{
+ return 0;
+}
diff --git a/src/thread/i386/clone.s b/src/thread/i386/clone.s
index 52fe7ef..f864231 100644
--- a/src/thread/i386/clone.s
+++ b/src/thread/i386/clone.s
@@ -22,7 +22,8 @@ __clone:
and $-16,%ecx
sub $16,%ecx
mov 20(%ebp),%edi
- mov %edi,(%ecx)
+ /* the ss prefix is needed to support the separate stack segment feature: */
+ mov %edi,%ss:(%ecx)
mov 24(%ebp),%edx
mov %esp,%esi
mov 32(%ebp),%edi
--
2.7.4
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
