Linux Sandbox: LOG error if a previous SIGSYS handler exists.

Linux Sandbox: LOG error if a previous SIGSYS handler exists.

SIGSYS is a reserved signal on Linux for the seccomp-bpf sandbox.
If a previous handler for SIGSYS exists, log an error.

BUG=178166
R=markus@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221539

[jln (very slow on Chromium), 2013-09-05 19:35:10]

Markus, PTAL!

[Markus (顧孟勤), 2013-09-05 22:23:24]

lgtm

https://codereview.chromium.org/23960006/diff/1/sandbox/linux/seccomp-bpf/tra...
File sandbox/linux/seccomp-bpf/trap.cc (right):

https://codereview.chromium.org/23960006/diff/1/sandbox/linux/seccomp-bpf/tra...
sandbox/linux/seccomp-bpf/trap.cc:66: sa.sa_handler != SIG_DFL) {
SIG_IGN is probably also OK, but we can hold off on that. Let's see which
reports we get back.

On that note, how are we getting any of these reports? Users don't typically
look at the output from LOG(ERROR) and tell us about it.

https://codereview.chromium.org/23960006/diff/1/sandbox/linux/seccomp-bpf/tra...
sandbox/linux/seccomp-bpf/trap.cc:85: struct sigaction old_sa = { };
Why do you need to zero out this structure? I know, just a nitpick.

https://codereview.chromium.org/23960006/diff/1/sandbox/linux/seccomp-bpf/tra...
sandbox/linux/seccomp-bpf/trap.cc:92: LOG(ERROR) << "Existing signal handler
when trying to install SIGSYS";
It would be nice, if we could actually log what the handler points to. But
that's a lot more complicated.

[jln (very slow on Chromium), 2013-09-05 22:29:05]

https://codereview.chromium.org/23960006/diff/1/sandbox/linux/seccomp-bpf/tra...
File sandbox/linux/seccomp-bpf/trap.cc (right):

https://codereview.chromium.org/23960006/diff/1/sandbox/linux/seccomp-bpf/tra...
sandbox/linux/seccomp-bpf/trap.cc:66: sa.sa_handler != SIG_DFL) {
On 2013/09/05 22:23:25, Markus (顧孟勤) wrote:
> SIG_IGN is probably also OK, but we can hold off on that. Let's see which
> reports we get back.
> 
> On that note, how are we getting any of these reports? Users don't typically
> look at the output from LOG(ERROR) and tell us about it.

It'll mostly be for the spurious SIGSYS bug report(s) for the time being.

Hopefully, soon we can DCHECK and "lock this in".

https://codereview.chromium.org/23960006/diff/1/sandbox/linux/seccomp-bpf/tra...
sandbox/linux/seccomp-bpf/trap.cc:85: struct sigaction old_sa = { };
On 2013/09/05 22:23:25, Markus (顧孟勤) wrote:
> Why do you need to zero out this structure? I know, just a nitpick.

Done.

https://codereview.chromium.org/23960006/diff/1/sandbox/linux/seccomp-bpf/tra...
sandbox/linux/seccomp-bpf/trap.cc:92: LOG(ERROR) << "Existing signal handler
when trying to install SIGSYS";
On 2013/09/05 22:23:25, Markus (顧孟勤) wrote:
> It would be nice, if we could actually log what the handler points to. But
> that's a lot more complicated.

Yeah, and in the spurious SIGSYS case, it's likely that we won't have symbols
anyways (binary driver), even if we implemented symbolization support.

[jln (very slow on Chromium), 2013-09-05 23:25:55]

Committed patchset #2 manually as r221539 (presubmit successful).