Jeff-Lewis / address-sanitizer

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

Correctly handle the symbolizer pipe on fork() #253

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
(as discussed)

Original issue reported on code.google.com by earth...@google.com on 16 Dec 2013 at 1:58

GoogleCodeExporter commented 9 years ago
To summarize the discussion, after the client program forks both the child and 
the parent retain pipes to the symbolizer process, and the data for the two 
processes sent/received by the symbolizer will be mixed.
An easy fix will be to just close the symbolizer pipes in the child so that a 
new symbolizer is created lazily when the next report is printed.
We can also extend the symbolizer API with a command that'll spawn a new 
symbolizer thread and return two fds to interact with it. This way we'll keep a 
single symbolizer process and save startup time.

Original comment by ramosian.glider@gmail.com on 16 Dec 2013 at 2:43

GoogleCodeExporter commented 9 years ago
Currently symbolizer set up pipes and spawns new subprocess "on demand". We can 
indeed implement an "easy fix" and add a method that would close the pipes, and 
then call it in the child subprocess immediately after fork (though, looks like 
this machinery is available only in TSan). I don't fully understand the second 
suggested approach - it's kind of dangerous to spawn new threads in programs 
that could potentially be forked, isn't it? The symbolizer *does* grab a lock.

Original comment by samso...@google.com on 8 May 2014 at 11:45

GoogleCodeExporter commented 9 years ago
I think glider is talking about threads in the symbolizer process.

Original comment by earth...@google.com on 12 May 2014 at 10:47

GoogleCodeExporter commented 9 years ago
Instead of a pipe you can use a SOCK_SEQPACKET socket to communicate between 
the symbolizer and the parent process. Then the child processes can simply use 
the same fd.

I know you can do many-to-one communication in this way, but I'm not sure about 
one-to-many. I think it's possible with sendto(). Will have to check.

Original comment by earth...@google.com on 12 May 2014 at 11:14

GoogleCodeExporter commented 9 years ago
Is there a real-life example of situation when we have many (more than 5) 
forked processes, and each of them needs to print symbolized error report from 
sanitizer? If it's not the case, I think we can get away with spawning a 
separate symbolizer for each subprocess.

Original comment by samso...@google.com on 12 May 2014 at 9:38

GoogleCodeExporter commented 9 years ago
For Chromium we need to prefork a single symbolizer before the sandbox is 
turned on (new processes may appear after that, but newly forked symbolizers 
won't be able to open files).
The idea is to keep an open socket to the symbolizer that a new Chrome process 
will use to request an exclusive pipe/socketpair to interact with the 
symbolizer process.

It actually doesn't matter how many threads we're going to have in the 
symbolizer.

Original comment by ramosian.glider@gmail.com on 13 May 2014 at 9:40

GoogleCodeExporter commented 9 years ago
If you look at the __sanitizer_sandbox_on_notify() call site in Chromium, it 
actually says it "should not fork".

https://code.google.com/p/chromium/codesearch#chromium/src/content/common/sandbo
x_linux/sandbox_linux.cc&l=140

We should still ask the security team about this though.

Original comment by earth...@chromium.org on 13 May 2014 at 11:28

GoogleCodeExporter commented 9 years ago

Original comment by ramosian.glider@gmail.com on 30 Jul 2015 at 9:05

GoogleCodeExporter commented 9 years ago
Adding Project:AddressSanitizer as part of GitHub migration.

Original comment by ramosian.glider@gmail.com on 30 Jul 2015 at 9:06