ASan should detect thread-hostile fork() usage

[ramosian-glider]

Originally reported on Google Code with ID 135

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().

Reported by ramosian.glider on 2012-12-12 08:30:49

[ramosian-glider]

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.

Reported by rnk@google.com on 2012-12-12 14:53:23

[ramosian-glider]

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.

Reported by decoder.oh on 2012-12-12 14:58:25

[ramosian-glider]

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? 

Reported by konstantin.s.serebryany on 2012-12-12 15:07:42

[ramosian-glider]

I can't really answer that, I haven't tried and never used tcmalloc before either.

Reported by decoder.oh on 2012-12-12 15:13:57

[ramosian-glider]

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.

Reported by timurrrr@google.com on 2012-12-12 17:47:32

[ramosian-glider]

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.

Reported by rnk@google.com on 2012-12-12 18:03:58

[ramosian-glider]

> 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

Reported by timurrrr@google.com on 2012-12-12 18:40:31

[ramosian-glider]

Dima has implemented this feature in TSan, not sure we still want the same functionality
for ASan.

Reported by ramosian.glider on 2014-05-07 09:04:15

• Status changed: WontFix

[ramosian-glider]

Adding Project:AddressSanitizer as part of GitHub migration.

Reported by ramosian.glider on 2015-07-30 09:13:40

• Labels added: ProjectAddressSanitizer