From 50c79657b611ac92cbe54dc94ecb2f60d802688b Mon Sep 17 00:00:00 2001 From: "jln@chromium.org" Date: Wed, 4 Sep 2013 21:33:22 +0000 Subject: Linux sandbox: do not crash on spurious SIGSYS. SIGSYS is a reserved signal for sandboxing on Linux. When we receive a spurious SIGSYS, we typically crash. This patch changes this behavior to only log an error. BUG=178166 R=markus@chromium.org Review URL: https://codereview.chromium.org/23686010 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@221274 0039d316-1c4b-4281-b951-d872f2087c98 --- sandbox/linux/seccomp-bpf/trap.cc | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/sandbox/linux/seccomp-bpf/trap.cc b/sandbox/linux/seccomp-bpf/trap.cc index 78a78ee..3c30de35 100644 --- a/sandbox/linux/seccomp-bpf/trap.cc +++ b/sandbox/linux/seccomp-bpf/trap.cc @@ -111,26 +111,24 @@ void Trap::SigSysAction(int nr, siginfo_t *info, void *void_context) { } void Trap::SigSys(int nr, siginfo_t *info, void *void_context) { + // Signal handlers should always preserve "errno". Otherwise, we could + // trigger really subtle bugs. + const int old_errno = errno; + // Various sanity checks to make sure we actually received a signal // triggered by a BPF filter. If something else triggered SIGSYS // (e.g. kill()), there is really nothing we can do with this signal. if (nr != SIGSYS || info->si_code != SYS_SECCOMP || !void_context || info->si_errno <= 0 || static_cast(info->si_errno) > trap_array_size_) { - // SANDBOX_DIE() can call LOG(FATAL). This is not normally async-signal - // safe and can lead to bugs. We should eventually implement a different - // logging and reporting mechanism that is safe to be called from - // the sigSys() handler. - // TODO: If we feel confident that our code otherwise works correctly, we - // could actually make an argument that spurious SIGSYS should - // just get silently ignored. TBD - SANDBOX_DIE("Unexpected SIGSYS received."); + // ATI drivers seem to send SIGSYS, so this cannot be FATAL. + // See crbug.com/178166. + // TODO(jln): add a DCHECK or move back to FATAL. + RAW_LOG(ERROR, "Unexpected SIGSYS received."); + errno = old_errno; + return; } - // Signal handlers should always preserve "errno". Otherwise, we could - // trigger really subtle bugs. - const int old_errno = errno; - // Obtain the signal context. This, most notably, gives us access to // all CPU registers at the time of the signal. ucontext_t *ctx = reinterpret_cast(void_context); @@ -145,6 +143,11 @@ void Trap::SigSys(int nr, siginfo_t *info, void *void_context) { if (sigsys.ip != reinterpret_cast(SECCOMP_IP(ctx)) || sigsys.nr != static_cast(SECCOMP_SYSCALL(ctx)) || sigsys.arch != SECCOMP_ARCH) { + // TODO(markus): + // SANDBOX_DIE() can call LOG(FATAL). This is not normally async-signal + // safe and can lead to bugs. We should eventually implement a different + // logging and reporting mechanism that is safe to be called from + // the sigSys() handler. SANDBOX_DIE("Sanity checks are failing after receiving SIGSYS."); } -- cgit v1.1