Currently, if a system call is not in the seccomp whitelist, we log the system call number and arguments, and they can also be obtained from the registers saved in the crash dump. However, none of that is visible on crash-stats, unless the viewer either has permission to download the raw crash dump and analyzes it locally, or they happen to have a matching copy of libc.so to resolve the offset. Possibly more to the point, it isn't searchable on crash-stats. Ideally the libc symbols would include the name of the function that made the system call, but the breakpad symbol dumper uses DWARF only and ignores the ELF symbol table entry (and these functions don't have DWARF tags, because they're written in assembly), and fixing this isn't trivial — and even if we did fix it that way, the fix would be in the same situation as bug 942290, where crash-stats uses some other build's symbols for the same libc.so instead. Also, the preceding paragraph applies only to B2G; other platforms we don't have symbols for libc at all, as I understand it. And we may also not have symbols for the immediate caller (if it's a proprietary library on B2G, or any system library otherwise) to identify the call site. But we could also expose the system call number via a field that's public, like si_addr ("crash address", currently the same as the signal context's PC, thus uninformative), or with an explicit note. As for the argument registers, I'm concerned that they might potentially contain privacy-sensitive data in the general case, but for specific cases where we're filtering on an argument (e.g., ioctl()'s request selector) it might be valuable to have it visible/searchable (and not be more revealing than the stack trace).
This is a bigger problem on x86, because the register with the syscall number (rAX) is apparently not in the crash dump. I'd like to have an answer for this before we ship seccomp-bpf on x86.
CrashReporter::AnnotateCrashReport is limited to the main thread and works by sending an IPC message to the parent, which rules it out for this. It'll have to be si_addr.
Tested on desktop with --enable-content-sandbox.
Comment on attachment 8461722 [details] [diff] [review] bug1017393-note-bad-syscall-hg0.diff As discussed on IRC, this is fine, the crash address is redundant for non-SEGV crashes anyway (it's just the faulting instruction pointer).
Attachment #8461722 - Flags: feedback?(ted) → feedback+
Comment on attachment 8461722 [details] [diff] [review] bug1017393-note-bad-syscall-hg0.diff Review of attachment 8461722 [details] [diff] [review]: ----------------------------------------------------------------- From my POV this is completely fine
Attachment #8461722 - Flags: review?(gdestuynder) → review+
https://tbpl.mozilla.org/?tree=Try&rev=46db8de2032d (also tested locally as noted earlier).
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Approval Request Comment [Feature/regressing bug #]: Bug 1012951 [User impact if declined]: It will be more difficult to analyze crash reports if/when sandbox failures happen. [Describe test coverage new/current, TBPL]: Tested locally. The testsuite doesn't include deliberate sandbox violations (yet). [Risks and why]: None; patch is very simple. [String/UUID change made/needed]: None.
Attachment #8473926 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.