seccomp-b2g support for ICS emulator / on TBPL

RESOLVED FIXED

Status

Firefox OS
General
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jld, Assigned: jld)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
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....
(Assignee)

Updated

4 years ago
Component: General → General
Product: Firefox → Firefox OS
Version: Trunk → unspecified
(Assignee)

Updated

4 years ago
Depends on: 932104
(Assignee)

Updated

4 years ago
Whiteboard: [c= p= s= u= ]
(Assignee)

Updated

4 years ago
Depends on: 941375

Updated

4 years ago
Whiteboard: [c= p= s= u= ]
(Assignee)

Updated

4 years ago
Depends on: 936320
(Assignee)

Updated

4 years ago
Summary: seccomp-b2g support for ICS emulator → seccomp-b2g support for ICS emulator / on TBPL
(Assignee)

Updated

4 years ago
Depends on: 962769
(Assignee)

Updated

4 years ago
Depends on: 966547
(Assignee)

Updated

4 years ago
No longer depends on: 966547
(Assignee)

Comment 1

4 years ago
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
No longer blocks: 790923
(Assignee)

Comment 2

4 years ago
Created attachment 8379976 [details] [diff] [review]
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.
Attachment #8379976 - Flags: review?(gdestuynder)
(Assignee)

Comment 3

4 years ago
Created attachment 8379977 [details] [diff] [review]
9999-Enable-SECCOMP_FILTER-on-goldfish-Android-emulator.patch
Attachment #8379977 - Flags: review?(gdestuynder)
Attachment #8379977 - Flags: review?(gdestuynder) → review+
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
Attachment #8379976 - Flags: review?(gdestuynder) → review+
(Assignee)

Comment 5

4 years ago
(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
(Assignee)

Updated

4 years ago
Attachment #8379977 - Attachment description: 0002-Enable-SECCOMP_FILTER-on-goldfish-Android-emulator.patch → 9999-Enable-SECCOMP_FILTER-on-goldfish-Android-emulator.patch
(Assignee)

Comment 6

4 years ago
Created attachment 8380756 [details] [diff] [review]
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
Attachment #8380756 - Flags: review?(gdestuynder)
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 :)
Attachment #8380756 - Flags: review?(gdestuynder) → review+
(Assignee)

Updated

4 years ago
Attachment #8379976 - Attachment is obsolete: true
(Assignee)

Comment 8

4 years ago
Created attachment 8380914 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/kernel_goldfish/pull/6

Pull request for source.  Carrying over r=kang.
Attachment #8380914 - Flags: review+
(Assignee)

Comment 9

4 years ago
Created attachment 8380917 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/platform_prebuilts_qemu-kernel/pull/4

Pull request for binaries.  Carrying over r=kang.
Attachment #8380917 - Flags: review+
(Assignee)

Comment 10

4 years ago
https://github.com/mozilla-b2g/kernel_goldfish/commit/520c1d010f9d0f4932aa0b24c413c0487c5345c4
https://github.com/mozilla-b2g/platform_prebuilts_qemu-kernel/commit/431afac2ebfdd9c1c8402b413ff5914ed448e961
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Comment 11

4 years ago
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.)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 12

4 years ago
https://hg.mozilla.org/mozilla-central/rev/eb03401f4af6
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.