Skip to contentSkip to conversation

cve/stack_clash: inhibit test-breaking optimizations

PATCH New Greg Hackmann 2 participants 2 messages Dec 4, 2018 – Dec 5, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cve/stack_clash: inhibit test-breaking optimizations

stack_clash relies on the compiler doing fairly specific things with the stack pointer. We've run into a couple of places where clang optimized away the intended behavior:

(1) exhaust_stack_into_sigsegv() tries to grow the stack via infinite recursion. Since this happens through a tail call, the compiler may transform this into an iterative equivalent which loops forever without growing the stack. -fno-optimize-sibling-calls explicitly blocks this optimization on both gcc and clang.

(2) If preallocate_stack() is inlined, its ~1 MB stack frame may not be released until the parent function returns. This can cause the stack pointer to cross the stack guard in the middle of do_child(), before the SIGSEGV handler has actually been set up. Prevent this by marking the function noinline.

---
 testcases/cve/Makefile      | 2 ++
 testcases/cve/stack_clash.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)
testcases/cve/Makefile +2 -0
index 3a05dd4fe..6dc401cff 100644
@@ -19,6 +19,8 @@ include $(top_srcdir)/include/mk/testcases.mk
 
 CFLAGS			+= -D_GNU_SOURCE
 
+stack_clash:	CFLAGS += -fno-optimize-sibling-calls
+
 cve-2016-7042:	LDLIBS += $(KEYUTILS_LIBS)
 
 cve-2016-7117:	CFLAGS += -pthread
testcases/cve/stack_clash.c +1 -1
index f25e19b3f..ac4a31af6 100644
@@ -136,7 +136,7 @@ void dump_proc_self_maps(void)
 	tst_run_cmd(cmd, NULL, NULL, 0);
 }
 
-void preallocate_stack(unsigned long required)
+void __attribute__((noinline)) preallocate_stack(unsigned long required)
 {
 	volatile char *garbage;
 
Signature · 1 line
2.20.0.rc1.387.gf8505762e3-goog
#1 lore raw

PATCH 1/1 cve/stack_clash: inhibit test-breaking optimizations

Hi Greg,

Quoted 3 lines

stack_clash relies on the compiler doing fairly specific things with the stack pointer. We've run into a couple of places where clang optimized away the intended behavior:

...

Quoted 5 lines
---
 testcases/cve/Makefile      | 2 ++
 testcases/cve/stack_clash.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

Thanks for your patch, pushed.

Kind regards,
Petr