Closed Bug 932098 Opened 6 years ago Closed 6 years ago

seccomp-b2g support for ICS emulator / on TBPL


(Firefox OS Graveyard :: General, defect)

Gonk (Firefox OS)
Not set


(Not tracked)



(Reporter: jld, Assigned: jld)




(4 files, 1 obsolete file)

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....
Product: Firefox → Firefox OS
Version: Trunk → unspecified
Whiteboard: [c= p= s= u= ]
Whiteboard: [c= p= s= u= ]
Summary: seccomp-b2g support for ICS emulator → seccomp-b2g support for ICS emulator / on TBPL
No longer depends on: 966547
A workaround for bug 936320 landed; it's leave-open pending a better fix, but the workaround is good enough for this.
No longer depends on: 936320
This patch is large, but it's mostly the same as and the parts where I had to make nontrivial changes are noted in the commit message.
Attachment #8379976 - Flags: review?(gdestuynder)
Comment on attachment 8379976 [details] [diff] [review]

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;


::: 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)

		A = seccomp_bpf_load(fentry->k);
Attachment #8379976 - Flags: review?(gdestuynder) → review+
(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.


> ::: 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.

Attachment #8379977 - Attachment description: 0002-Enable-SECCOMP_FILTER-on-goldfish-Android-emulator.patch → 9999-Enable-SECCOMP_FILTER-on-goldfish-Android-emulator.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:
Attachment #8380756 - Flags: review?(gdestuynder)
Comment on attachment 8380756 [details] [diff] [review]

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

*eyes bleeding*
looks good tho :)
Attachment #8380756 - Flags: review?(gdestuynder) → review+
Attachment #8379976 - Attachment is obsolete: true
Pull request for source.  Carrying over r=kang.
Attachment #8380914 - Flags: review+

(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.)
Resolution: FIXED → ---
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.