932098 - seccomp-b2g support for ICS emulator / on TBPL

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

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

It looks like we're going to be living with ICS for a while, so it'd be nice if the emulator — which is the device everyone has access to, and what TBPL runs tests on — could be used to test sandboxing.

Problem: the ICS emulator is using an even older Linux kernel (2.6.29) than real ICS.

But I've managed to backport the seccomp-bpf patches (our 3.0 version, originally from 3.5), despite nontrivial changes to the BPF engine.  The result probably has local DoS (or worse) if the filter program itself is hostile, but it does seem to work, and for testing/development purposes it should be fine.

I'd like to clean up the patches a little more before asking for comments.  Also, if we push a new kernel to our platform_prebuilts_qemu-kernel repo, I think that will cause TBPL to start using it, so that can't happen until tests pass (and any developer-quality-of-life issues we think are essential are addressed).  Dependent bugs to follow....

Comment 1

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

A workaround for bug 936320 landed; it's leave-open pending a better fix, but the workaround is good enough for this.

Comment 2

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

Attachment: 0001-Backport-of-Seccomp-bpf-mode-filter-from-Linux.patch

This patch is large, but it's mostly the same as https://github.com/gp-b2g/gp-keon-kernel/commit/ac3566aa70b1 and the parts where I had to make nontrivial changes are noted in the commit message.

Comment 3

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

Attachment: 9999-Enable-SECCOMP_FILTER-on-goldfish-Android-emulator.patch

Comment 4

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

Comment on attachment 8379976 [details] [diff] [review]
0001-Backport-of-Seccomp-bpf-mode-filter-from-Linux.patch

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

Looks fine to me. Theres the BPF_S_ANC_SECCOMP_LD_W stuff missing but i suppose you had reasons for not including it.
I also looked at seccomp_run_filters and I assume it works fine - at least, it looks ok.

::: include/linux/filter.h
@@ +140,3 @@
>  struct sk_filter
>  {
>  	atomic_t		refcnt;

No BPF_S_ANC_SECCOMP_LD_W, ? (http://www.gossamer-threads.com/lists/linux/kernel/1504443?do=post_view_threaded#1504443)

::: kernel/seccomp.c
@@ +85,5 @@
> + * @syscall: number of the current system call
> + *
> + * Returns valid seccomp BPF response codes.
> + */
> +static u32 seccomp_run_filters(struct pt_regs *regs)

(just adding a comment here to remember that the major diff in the seccomp code is here)

::: kernel/signal.c
@@ +2145,1 @@
>  		err |= __put_user(from->si_uid, &to->si_uid);

No #define SYNCHRONOUS_MASK  I guess?

::: net/core/filter.c
@@ +40,3 @@
>  
>  /* No hurry in this branch */
>  static void *__load_pointer(struct sk_buff *skb, int k)

Again no 	#ifdef CONFIG_SECCOMP_FILTER
		case BPF_S_ANC_SECCOMP_LD_W:
		A = seccomp_bpf_load(fentry->k);
		continue;
		#endif

Comment 5

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

(In reply to Guillaume Destuynder [:kang] (use NEEDINFO!) from comment #4)
> Looks fine to me. Theres the BPF_S_ANC_SECCOMP_LD_W stuff missing but i
> suppose you had reasons for not including it.

This kernel predates [01f2f3f], which added the BPF_S_* enum values and the code that rewrites the BPF opcodes into them.  It was introduced as an optimization (so that the switch statement over that enum would compile into something more efficient than the code to decode the BPF opcodes), but once it was there, the seccomp-bpf patch added special enum values for seccomp and modified the translation to turn BPF_LD into them instead of the regular BPF_S_LD (which would read the nonexistent network packet).

This is why I removed it and added the fake_packet code instead.

[01f2f3f]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=01f2f3f6ef4d076c0c10a8a7b42624416d56b523

> ::: kernel/signal.c
> @@ +2145,1 @@
> >  		err |= __put_user(from->si_uid, &to->si_uid);
> 
> No #define SYNCHRONOUS_MASK  I guess?

Indeed not.  That's from [a27341c], which changes which signal is delivered first if multiple signals are pending.

[a27341c]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a27341cd5fcb7cf2d2d4726e9f324009f7162c00

Comment 6

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

Attachment: 0001-Backport-of-Seccomp-bpf-mode-filter-from-Linux.patch

It turns out that 01f2f3f6ef4d ("net: optimize Berkeley Packet Filter (BPF) processing"), mentioned previously, cherry-picks cleanly onto the 2.6.29 kernel for the ICS emulator.

So, this is a cleaner version of the seccomp-bpf patch, without the fake_packet stuff and with the BPF_S_ANC_SECCOMP_LD_W/seccomp_check_filter code restored; it applies on top the cherry-picked 01f2f3f6ef4d.

I've done basic tests locally.  Try: https://tbpl.mozilla.org/?tree=Try&rev=7f87582f4428

Comment 7

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

Comment on attachment 8380756 [details] [diff] [review]
0001-Backport-of-Seccomp-bpf-mode-filter-from-Linux.patch

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

*eyes bleeding*
looks good tho :)

Comment 8

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

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/kernel_goldfish/pull/6

Pull request for source.  Carrying over r=kang.

Comment 9

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

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/platform_prebuilts_qemu-kernel/pull/4

Pull request for binaries.  Carrying over r=kang.

Comment 10

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

https://github.com/mozilla-b2g/kernel_goldfish/commit/520c1d010f9d0f4932aa0b24c413c0487c5345c4
https://github.com/mozilla-b2g/platform_prebuilts_qemu-kernel/commit/431afac2ebfdd9c1c8402b413ff5914ed448e961

Comment 11

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

https://hg.mozilla.org/integration/b2g-inbound/rev/eb03401f4af6

(Also, reopening until this lands on m-c for releng builds, even though the commits in comment #10 will affect a local build of m-c immediately.)

Comment 12

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

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