gopalshankar / address-sanitizer

Automatically exported from code.google.com/p/address-sanitizer
0 stars 0 forks source link

ASan should detect thread-hostile fork() usage #135

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
If an application calls fork() in the presence of several threads, all of those 
threads except the current one disappear. If any locks have been acquired in 
other threads prior to fork(), they remain acquired, so the remaining thread 
should not attempt to take them. This may lead to deadlocks, which are hard to 
debug.
It may help to wrap fork() in ASan and forbid taking locks (including calling 
malloc()) in the child process. This may be too strict (therefore should be put 
under a flag), but is a good practice to enforce.
Note that on Mac OS the client application is usually responsible for locking 
and unlocking the malloc zone around the call to fork().

Original issue reported on code.google.com by ramosian.glider@gmail.com on 12 Dec 2012 at 8:30

GoogleCodeExporter commented 9 years ago
In particular, doesn't glibc make sure to acquire any malloc locks before 
forking?  Should the ASan RT do the same when this flag is disabled?  Apps that 
only care about running on glibc may be OK with using malloc after fork before 
exec, and it would be nice if the ASan RT didn't introduce bugs that aren't 
there with glibc.

Original comment by rnk@google.com on 12 Dec 2012 at 2:53

GoogleCodeExporter commented 9 years ago
Fwiw, I recently already sent this to the list: On Firefox we are currently 
experiencing a deadlock situation when ASan triggers and the stack of the 
remaining locked process contains malloc, asan_malloc and fork. Not sure if 
this is related but it sounds like it could be.

Original comment by decoder...@googlemail.com on 12 Dec 2012 at 2:58

GoogleCodeExporter commented 9 years ago
OMG

what if a program uses tcmalloc?
Do we also have to fix tcmalloc to release its internal locks after fork?

decoder.oh: does Firefox work with tcmalloc? 

Original comment by konstant...@gmail.com on 12 Dec 2012 at 3:07

GoogleCodeExporter commented 9 years ago
I can't really answer that, I haven't tried and never used tcmalloc before 
either.

Original comment by decoder...@googlemail.com on 12 Dec 2012 at 3:13

GoogleCodeExporter commented 9 years ago
Re: comment #1
I think the problem with this approach is that you're doomed as soon as you 
have some legacy / third party code that does something that this pre-fork hack 
doesn't know about.

Original comment by timurrrr@google.com on 12 Dec 2012 at 5:47

GoogleCodeExporter commented 9 years ago
Timur, I don't understand.  You mean some other third party lock gets acquired 
post fork, pre execve?  I absolutely agree that asan should warn on that.  The 
whole pthread_atfork() thing is a hacky mess, and it's usually easy to avoid 
calling libraries like that.  I'm only suggesting special casing malloc.

I just worry that there is just too much code out there that calls malloc after 
fork.  Basically any C++ programmer that doesn't understand fork will use 
std::vector<>::push_back to construct argv, for example.

I just don't like the idea that switching to asan RT can take a program that 
doesn't deadlock and make it deadlock.  Calling malloc after fork is basically 
crazy and not portable.  Maybe warning the user is enough.

Original comment by rnk@google.com on 12 Dec 2012 at 6:03

GoogleCodeExporter commented 9 years ago
> I just worry that there is just too much code out there that calls malloc 
after fork.
> Basically any C++ programmer that doesn't understand fork will use
> std::vector<>::push_back to construct argv, for example.
He's in trouble anyways as soon as he switches his memory allocator?

I teach my students to use vector before fork =P

Original comment by timurrrr@google.com on 12 Dec 2012 at 6:40

GoogleCodeExporter commented 9 years ago
Dima has implemented this feature in TSan, not sure we still want the same 
functionality for ASan.

Original comment by ramosian.glider@gmail.com on 7 May 2014 at 9:04