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;
}

Attachment: safe-unwind-1.patch
Description: Binary data

Reply via email to