[<prev] [next>] [day] [month] [year] [list]
Message-ID: <390CE752059EB848A71F4F676EBAB76D3AC263D2@ORSMSX114.amr.corp.intel.com>
Date: Fri, 28 Oct 2016 20:13:25 +0000
From: "LeMay, Michael" <michael.lemay@...el.com>
To: "musl@...ts.openwall.com" <musl@...ts.openwall.com>
Subject: [RFC PATCH v2 1/2] add support for 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 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 ++++++-
configure | 10 +++
src/env/__libc_start_main.c | 6 ++
src/internal/i386/separate_stack_seg.c | 151 +++++++++++++++++++++++++++++++++
src/internal/safe_stack.c | 25 +++++-
src/thread/i386/clone.s | 3 +-
6 files changed, 219 insertions(+), 7 deletions(-)
create mode 100644 src/internal/i386/separate_stack_seg.c
diff --git a/Makefile b/Makefile
index 0bc51ac..28c17ce 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,8 +134,9 @@ 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)
-# The safestack attribute will be selectively forced within the __init_tls.c file below.
-SAFE_STACK_OBJS = $(filter-out $(CRT_OBJS) obj/ldso/dlstart.o obj/src/env/__init_tls.o,$(ALL_OBJS))
+SEP_STACK_SEG_OBJS = $(filter-out $(CRT_OBJS) obj/ldso/dlstart.o,$(ALL_OBJS))
+# The safestack attribute will be selectively forced within this file below.
+SAFE_STACK_OBJS = $(filter-out obj/src/env/__init_tls.o,$(SEP_STACK_SEG_OBJS))
ifeq ($(SAFE_STACK),yes)
@@ -172,6 +173,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 X86FixupSeparateStackSeg 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 X86FixupSeparateStackSeg 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/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 b3c2b0d..a2c67a3 100644
--- a/src/env/__libc_start_main.c
+++ b/src/env/__libc_start_main.c
@@ -71,6 +71,9 @@ static void libc_start_init(void)
weak_alias(libc_start_init, __libc_start_init);
+#if SEP_STACK_SEG
+void __safestack_dup_argv_env_aux(int argc, char ***argvp, char ***envpp);
+#endif
#if SAFE_STACK
void __preinit_unsafe_stack(void);
@@ -84,6 +87,9 @@ int __libc_start_main(int (*main)(int,char **,char **), int argc, char **argv)
__preinit_unsafe_stack();
#endif
__init_libc(envp, argv[0]);
+#if SEP_STACK_SEG
+ __safestack_dup_argv_env_aux(argc, &argv, &envp);
+#endif
__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..acc73f4
--- /dev/null
+++ b/src/internal/i386/separate_stack_seg.c
@@ -0,0 +1,151 @@
+#if SEP_STACK_SEG
+
+#define _GNU_SOURCE
+#include "atomic.h"
+#include "libc.h"
+#include "syscall.h"
+#include <ldt.h>
+#include <malloc.h>
+#include <stdint.h>
+#include <string.h>
+
+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(int idx, unsigned long addr, size_t len) {
+ struct user_desc stack_desc;
+ stack_desc.entry_number = idx;
+ stack_desc.base_addr = (unsigned long)addr;
+ 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 int find_available_ldt_slot(void) {
+ static const int SEG_DESC_P_BIT_OFF = 47;
+
+ uint64_t ldt[1];
+
+ /* read the current LDT */
+ int ldt_len = modify_ldt(0, ldt, sizeof(ldt));
+ if (ldt_len == -1)
+ a_crash();
+
+ if (ldt_len == 0)
+ /* LDT is currently empty */
+ return 0;
+
+ for (int i = 0; i < (ldt_len/sizeof(ldt[0])); i++)
+ if (((ldt[i] >> SEG_DESC_P_BIT_OFF) & 1) == 0)
+ return i;
+
+ /* crash if no available slot was found */
+ a_crash();
+
+ /* unreachable; just for suppressing compiler warning */
+ return 0;
+}
+
+#define SEG_SEL_LDT 4
+#define SEG_SEL_CPL3 3
+#define SEG_SEL_TO_IDX(sel) ((sel) >> 3)
+#define SEG_IDX_TO_LDT_SEL(idx) ((idx) | SEG_SEL_LDT | SEG_SEL_CPL3)
+
+__attribute__((__visibility__("hidden")))
+void __restrict_segments(void)
+{
+ uintptr_t limit, stack_base = __stack_base;
+ int data_seg_sel, data_ldt_sel;
+
+ __asm__ __volatile__ ("mov %%ds, %0" : "=r"(data_seg_sel));
+
+ if ((data_seg_sel & SEG_SEL_LDT) == SEG_SEL_LDT) {
+ data_ldt_sel = data_seg_sel;
+
+ /* 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
+ data_ldt_sel = SEG_IDX_TO_LDT_SEL(find_available_ldt_slot());
+
+ update_ldt(SEG_SEL_TO_IDX(data_ldt_sel), 0, 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"(data_ldt_sel)
+ );
+}
+
+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 __safestack_dup_argv_env_aux(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();
+}
+
+#endif /*SEP_STACK_SEG*/
diff --git a/src/internal/safe_stack.c b/src/internal/safe_stack.c
index 7c873e3..5e266fa 100644
--- a/src/internal/safe_stack.c
+++ b/src/internal/safe_stack.c
@@ -8,10 +8,18 @@
static bool unsafe_stack_ptr_inited = false;
+/* minimum base address of all existing safe stacks */
+__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);
+#if SEP_STACK_SEG
+void __restrict_segments(void);
+#endif
+
/* 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
* that runs prior to __init_unsafe_stack allocating a new unsafe stack. This
@@ -39,7 +47,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;
@@ -88,7 +96,16 @@ 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 = 0;
+#if SEP_STACK_SEG
+ /* 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. */
+
+ try_map = __stack_base - size;
+#endif
+
+ 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 +118,10 @@ int __safestack_init_thread(struct pthread *restrict new, const pthread_attr_t *
new->safe_stack_size = size;
new->stack = map + size;
+#if SEP_STACK_SEG
+ __restrict_segments();
+#endif
+
return 0;
fail:
return EAGAIN;
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.
