1102209 - Remove use of CodeGen::JoinInstructions in the Linux sandboxing code.

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Bob Owen (:bobowen)]

In Chromium commit 91577b8fe31f32ad6a5fcbf30632e30d80278854 they removed the function CodeGen::JoinInstructions from sandbox/linux/seccomp-bpf/codegen.h and we currently use it in security/sandbox/linux/SandboxAssembler.cpp

This currently prevents us from updating past this commit.

Comment 1

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

I can get rid of the Deny() items in SandboxFilter.cpp by moving them into code in the SIGSYS handler, since at least some of that already needs to happen for bug 1088387 (and fake syscall failure isn't performance-critical), and then this is a simple matter of accumulating the SandboxAssembler::Condition objects corresponding to what's allowed and iterating them in reverse.

Comment 2

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

(In reply to Jed Davis [:jld] from comment #1)
> I can get rid of the Deny() items in SandboxFilter.cpp by moving them into
> code in the SIGSYS handler

On second thought, this can and should wait until after the Chromium sandbox compiler is in use, so that the sandbox policy can stay in one place instead of being divided between SandboxFilter.cpp and Sandbox.cpp (more so than it already is).

Comment 3

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

Attachment: bug1102209-sandbox-assemble-order-hg0.diff

See commit message for more explanation.

Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=a10b997475eb

Comment 4

[Guillaume Destuynder [:kang] [this account is no longer active]]

Comment on attachment 8531922 [details] [diff] [review]
bug1102209-sandbox-assemble-order-hg0.diff

Review of attachment 8531922 [details] [diff] [review]:
-----------------------------------------------------------------

s/progrem/program :) (commit msg)
Do you know if we have a better performance test for similar changes? In the past, it was mostly checking the average ms to return from syscalls, manually verified. I don't think there's a measurable difference in this case, but it would be nice to run a regular check I guess.

Comment 5

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

(In reply to Guillaume Destuynder [:kang] from comment #4)
> s/progrem/program :) (commit msg)

Thanks.

> Do you know if we have a better performance test for similar changes? In the
> past, it was mostly checking the average ms to return from syscalls,
> manually verified. I don't think there's a measurable difference in this
> case, but it would be nice to run a regular check I guess.

We don't need to care about this yet, because this doesn't change the filter program.  I've verified this by running with MOZ_SANDBOX_VERBOSE set and diffing the BPF disassembly; the only change is to the getpid() value used for tgkill.

Comment 6

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/a2ae4c0a26fe

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a2ae4c0a26fe