Hi all, Libgcc unwinder currently does not do any verification of pointers which it chases on stack. In practice this not so rarely causes segfaults when unwinding on corrupted stacks (e.g. when when trying to print diagnostic on fatal error) [1]. Ironically this usually happens in error reporting code which puzzles users.
I've attached one motivating example - with current libgcc it will abort inside unwinder when trying to access invalid address (0x0a0a0a0a). There is an old request to provide a safer version of _Unwind_Backtrace (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67336) that would check pointer validity prior to dereferencing. I've attached a draft patch to see if this approach is viable and hopefully get some advice. The change is rather straightforward: I add a new _Unwind_Backtrace_Checked which checks return value of msync on every potentially unsafe access (libunwind does something like this as well, although in a very imcomplete manner). To avoid paying for syscalls too often, I cache the last checked memory page. Potentially parsing /proc/$$/maps would allow for much faster verification but I didn't bother too much as new APIs are intended for reporting fatal errors where speed isn't an issue. The code is only implemented for DW2 unwinder (probably used on most targets). So my questions now are: 1) Would this feature considered useful i.e. will it be accepted for trunk once implementation is polished/tested? 2) Should I strive to implement it for all possible targets or DW2 would do for now? I don't have easy access to other platforms (ARM, C6x, etc.) so this may delay implementation. 3) Any suggestions/comments on this attached draft implementation? E.g. alternative syscalls to use (Andrew suggested futex), how many verified addresses to cache, whether I should verify unwind table accesses in addition to stack accesses, etc. -Y [1] The issue has been reported in the past e.g. "Adventure with Stack Smashing Protector" (http://site.pi3.com.pl/papers/ASSP.pdf).
#include <string.h> #include <stdio.h> struct _Unwind_Context; typedef int (*_Unwind_Trace_Fn)(struct _Unwind_Context *, void *vdata); extern int _Unwind_Backtrace(_Unwind_Trace_Fn trace, void * trace_argument); extern int _Unwind_Backtrace_Checked(_Unwind_Trace_Fn trace, void * trace_argument); extern void *_Unwind_GetIP (struct _Unwind_Context *context); int simple_unwind (struct _Unwind_Context *context, void *vdata) { printf("Next frame: "); void *pc = _Unwind_GetIP(context); printf("%p\n", pc); return 0; } #define noinline __attribute__((noinline)) noinline int foo() { // Clobber stack to provoke errors in unwinder int x; void *p = &x; asm("" :: "r"(p)); memset(p, 0xa, 128); printf("After clobbering stack\n"); int ret = _Unwind_Backtrace(simple_unwind, 0); printf("After unwind: %d\n", ret); return 0; } noinline int bar() { int x = foo(); return x + 1; } int main() { bar(); return 0; }
safe-unwind-1.patch
Description: Binary data