New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 696507 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

prevent unwanted fork()s from happening in chrome

Project Member Reported by primiano@chromium.org, Feb 27 2017

Issue description

Some thoughts as a followup to  Issue 695643  and Issue 693668 .

We ran into a very subtle bug there due to the fact that a third party library was fork()ing to implement a get_bool() method, hence in a place where we didn't expect.
fork()ing is generally bad because of multithreading, unless it happens in a controlled place, e.g. by the zygote at startup. 
One of the concrete risks, for instance, is that the fork() happens while another thread is doing a malloc() and hence holding the global allocator lock. In that case the forked process will hang at its first malloc call.

Now, the specific problem about gconf/dbus seems to have been fixed, but the lesson learned is: we just found out that third party libraries can fork() in very subtle ways and hence hang chrome. It is not unlikely that some other library will do the same in future (or maybe is already happening, who knows)

It would be great if we could have:
1. A way to prevent forks() from generally happen (i.e. turn them into CHECKs)
2. A way to allow them in a scope (something similar to ScopedAllowIO) for the cases when we feel it's safe to fork() (e.g. zygote)

1) Could be achieved by:
 Option A: interposing fork() and clone() like we do with malloc() and friends and hoping that the third-party libraries don't satically link libc or don't directly do a syscall(NR_clone)
 Option B: registering a pthread_atfork handler
(I prefer B if possible, symbol interposition is always tricky to get right)

Any takers?
 
Also somebody just pointed out privately that tcmalloc 2.1 has a mechanism [1] to deal with the fork scenario depicted above. Might be tricky and risky for stability to update tcmalloc though. Also, this would baind-aid the problem with malloc() but leave it open for any other library. So, given the cost that would take to update our tcmalloc fork (sorry for the pun), not sure having that is a major improvement.
IMHO the real problem is preventing unexpected fork()s.

[1] https://github.com/gperftools/gperftools/issues/499

Comment 2 by rsesek@chromium.org, Feb 27 2017

You could also install a seccomp filter on __NR_clone, set up a trap handler, and then validate that the caller is permitted. The problem is that children would then inherit/stack that seccomp policy, unless you did it after zygote creation.
Cc: thomasanderson@chromium.org
After my self shaming on another fork bug about always ignoring these problems, I captured the attached stacktrace on KDE.
bt.txt
4.5 KB View Download
Re #2: you talk like somebody who writes seecomp filters every other day :P Joking aside a bit worried about the side effects on forks(). If that works I'd try the pthread_atfork approach (but also I don't have time for that, so if anybody wants to go ahead that would be great).

Re #4: Do I read that correctly? is the GtkUi() ctor end up doing a fork+exec? What went wrong with the world? :( 
Yep, apparently fun things happen inside gtk2-engines-oxygen.
Well, let's look at the good side : at least it doesn't try to start a pulseaudio server :) 
(I somehow fear that these might become my last famous words) 
Cc: timbrown@chromium.org

Sign in to add a comment