Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(4789)

Issue 1004002: Handle crashes caused by SIGSYS (e.g., due to SECCOMP_RET_TRAP). (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 2 months ago by Jed Davis
Modified:
10 years, 1 month ago
CC:
google-breakpad-dev_googlegroups.com, markus, wad
Visibility:
Public.

Description

Context: we're using seccomp-bpf for multiprocess Gecko, and in particular we're
using SECCOMP_RET_TRAP on developer builds to raise SIGSYS, to allow debugging
the sandbox violation.  This patch causes breakpad to catch the SIGSYS and
handle it like other types of crash, so we can do something useful when sandbox
violations are discovered by our automated tests.

However, we can't just return from the SIGSYS handler to reraise it (as with
SIGSEGV), because the signal context's PC is the next instruction after the
system call instruction that caused it, so it has to be special-cased.

Note: this patch is based on the patch from issue 984002.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M src/client/linux/handler/exception_handler.cc View 3 chunks +14 lines, -7 lines 0 comments Download

Messages

Total messages: 10
Ted Mielczarek
Lei, here's another patch to this same code that I don't feel qualified to review. ...
10 years, 2 months ago #1
Jed Davis
Is there an ETA for this? We'd like to get feedback on whether this is ...
10 years, 2 months ago #2
Lei Zhang (chromium)
+markus and jln in case they want to review this. Sorry for not responding earlier, ...
10 years, 2 months ago #3
jln
Lei: can you add wad@chromium.org to this review please? As a general remark, this won't ...
10 years, 2 months ago #4
Lei Zhang (chromium)
+wad per request
10 years, 2 months ago #5
jln
FYI, the way we deal with "unexpected SIGSYS" in Chromium is by generating an access ...
10 years, 2 months ago #6
Jed Davis
On 2014/01/27 23:14:02, jln wrote: > Why should this be part of breakpad? Wouldn't you ...
10 years, 2 months ago #7
Jed Davis
On 2014/01/28 21:14:12, Jed Davis wrote: > > But... I think we don't actually need ...
10 years, 2 months ago #8
jln
On 2014/01/29 00:11:38, Jed Davis wrote: > On 2014/01/28 21:14:12, Jed Davis wrote: > > ...
10 years, 1 month ago #9
Ted Mielczarek
10 years, 1 month ago #10
Message was sent while issue was closed.
On 2014/02/04 05:39:02, jln wrote:
> Great! I didn't look closely and I'm a bit in a rush ATM, but if you think
> something should be added to breakpad to support this use case more
explicitly,
> this may make sense..

The minimal patch to support this is here: https://breakpad.appspot.com/1114003/

(I'll land that when I have a minute.)
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1004:630ec63f810e-tainted