ramosian-glider / sanitizer-issues

test
0 stars 0 forks source link

ASan should detect stack overflows #60

Closed ramosian-glider closed 9 years ago

ramosian-glider commented 9 years ago

Originally reported on Google Code with ID 60

==============================
$ cat stack.c 
#include <pthread.h>
#include <stdio.h>

void *ExhaustWorker(void *arg) {
  volatile char a[1 << 16 + 1];
  fprintf(stderr, "a: %p\n", a);
  int iarg = *(int*)arg - 1;
  if (iarg > 0) {
    return ExhaustWorker(&iarg);
  } else {
    return NULL;
  }
}

int main() {
  pthread_t t;
  int arg = 1000;
  pthread_create(&t, NULL, ExhaustWorker, (void*)&arg);
  pthread_join(t, 0);
  return 0;
}
==============================
$ clang -faddress-sanitizer stack.c -o stack

$ ASAN_OPTIONS=verbosity=1 ./stack 
==2904== Parsed ASAN_OPTIONS: verbosity=1
==2904== AddressSanitizer: libc interceptors initialized
|| `[0x200000000000, 0x7fffffffffff]` || HighMem    ||
|| `[0x140000000000, 0x1fffffffffff]` || HighShadow ||
|| `[0x120000000000, 0x13ffffffffff]` || ShadowGap  ||
|| `[0x100000000000, 0x11ffffffffff]` || LowShadow  ||
|| `[0x000000000000, 0x0fffffffffff]` || LowMem     ||
MemToShadow(shadow): 0x120000000000 0x123fffffffff 0x128000000000 0x13ffffffffff
red_zone=128
malloc_context_size=30
SHADOW_SCALE: 3
SHADOW_GRANULARITY: 8
SHADOW_OFFSET: 100000000000
==2904== T0: stack [0x7fff75f43000,0x7fff76743000) size 0x800000; local=0x7fff7674175c
==2904== AddressSanitizer Init done
==2904== T1: stack [0x7fe6e3b58000,0x7fe6e4359000) size 0x801000; local=0x7fe6e4357e8c
a: 0x7fe6e4337e00
a: 0x7fe6e42f7c40
...

a: 0x7fe6e34716c0
a: 0x7fe6e3431500
a: 0x7fe6e33f1340
a: 0x7fe6e33b1180
Segmentation fault

Reported by ramosian.glider on 2012-04-04 12:03:07

ramosian-glider commented 9 years ago
We've hit the stack overflow issue in Chrome (see crbug.com/120623), where the Debug
build of the browser crashed because of the several reasons:

 -- one of the threads had a size of PTHREAD_SIZE_MIN
 -- ASan bloats the stack frames notably
 -- the Debug build contained too many local vars, so the stack frames were even larger

On the other hand, the same problem may occur (and maybe already occurs) with larger
stacks if the recursion is deep and the stack frames are large enough.

Reported by ramosian.glider on 2012-04-04 12:10:17

ramosian-glider commented 9 years ago
Can we increase the thread stack size in Chrome, if we're running under ASan?

What are you suggestions? If we want to report such an error, we need to add some kind
of custom ASan guard page (before the actual guard page), poison its shadow at thread
start and unpoison at thread exit, and emit additional load instruction at the beginning
of each function (that reads the memory from the "last" variable allocated on stack
in this frame). (Did I got it right?)

Where do you plan to get space for ASan guard page from? Effectively reduce the stack
size asked by the program, or just allocate your own, larger stack for a new thread?
Will this be ok on Mac, where we don't control all thread creation routines?

Reported by samsonov@google.com on 2012-04-04 12:22:43

ramosian-glider commented 9 years ago
> Can we increase the thread stack size in Chrome, if we're running under ASan?
Yes, I've sent a CL for that. But I'm talking about detecting such cases, not preventing
them. A thread stack may be pre-allocated by the client and passed via pthread_attr_t,
which will be very hard to prevent.

> What are you suggestions? 
I'm thinking about poisoning the actual guard page (no additional pages are needed)
and checking the shadow of the stack-allocated variables before the stack frame is
set up (the first byte of each var is probably enough, or we can check every kPageSizeth
byte in the stack frame).
We can unpoison the guard page when we leave the thread callback, although this should
be unnecessary most of the time.

> Will this be ok on Mac, where we don't control all thread creation routines?
We'll just poison less shadow if we feel like not setting the guard pages for the worker
threads (we actually still want to).
This has nothing to do with thread creation: guard pages protect from stack overflows,
which happen strictly when the thread callback is being ran. So we can set  the guard
up and tear it down only when the thread callback is working.

Reported by ramosian.glider on 2012-04-04 12:39:20

ramosian-glider commented 9 years ago
Looking a bit deeper this is not easily doable.
If we're about to hit the stack guard, there probably is little space for us to execute
the error reporting function (and the signal handler, thus no error is reported). So
if we want to work in this direction, we'll really need to decrease the stack size
by adding our own stack guard.

Reported by ramosian.glider on 2012-04-04 16:10:15

ramosian-glider commented 9 years ago
I've implemented the use_sigaltstack flag (Clang r154092) that allows to handle the
signals on alternate stacks. It should help in detecting stack overflows.

Reported by ramosian.glider on 2012-04-06 12:06:57

ramosian-glider commented 9 years ago

Reported by ramosian.glider on 2012-09-13 13:42:07

ramosian-glider commented 9 years ago
Adding Project:AddressSanitizer as part of GitHub migration.

Reported by ramosian.glider on 2015-07-30 09:12:59