1762540 - Avoid using SECCOMP_RET_KILL_THREAD

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, enhancement, P1)

Description

[Gian-Carlo Pascutto [:gcp]]

Introduced in kernel 4.13 0466bdb99e8744bc9befa8d62a317f0fd7fd7421

Should be possible in a backwards-compat way:

    Asynchronous termination of a thread outside of the userspace thread
    library's knowledge is an unsafe operation that leaves the process in
    an inconsistent, corrupt, and possibly unrecoverable state.
....
    The new action is discoverable by userspace through either the new
    actions_avail sysctl or through the SECCOMP_GET_ACTION_AVAIL seccomp
    operation. If used without checking for availability, old kernels
    will treat RET_KILL_PROCESS as RET_KILL_THREAD (since the old mask
    will produce RET_KILL_THREAD).

Comment 1

[Gian-Carlo Pascutto [:gcp]]

This would solve problems where a sandbox violation makes new tabs hang due to dead locks, rather than producing a nice crash.

Comment 2

[Gian-Carlo Pascutto [:gcp]]

There was a question if SECCOMP_RET_TRAP could be invoked and then have that kill the process. I have no idea if that'd work.

Comment 3

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

SECCOMP_RET_KILL_THREAD is, as mentioned, bad because it results in mysterious hangs. (The first attempt at using seccomp-bpf with Gecko, on B2G, did that by default; that broke something I was testing, so I fixed it, and I ended up becoming the owner.)

SECCOMP_RET_KILL_PROCESS will at least kill the process, but it's not ideal, because we won't get a crash report.

The only use of SECCOMP_RET_KILL_THREAD I can see is in DefaultPanic, and we can override that externally via SetPanicFunc. I have a patch to do this, which I wrote while investigating bug 1759196, and it seems to work (with an artificial test case; I haven't tried to reproduce the original bug there with a patched build yet). So I'm going to take this bug and adjust the purpose a little.

Comment 4

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

Attachment: Bug 1762540 - Crash normally, instead of killing the calling thread, for certain sandbox errors.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4745b182f545
Crash normally, instead of killing the calling thread, for certain sandbox errors. r=gcp

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/4745b182f545