Closed
Bug 932098
Opened 11 years ago
Closed 11 years ago
seccomp-b2g support for ICS emulator / on TBPL
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jld, Assigned: jld)
References
Details
Attachments
(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....
Assignee | ||
Updated•11 years ago
|
Product: Firefox → Firefox OS
Version: Trunk → unspecified
Assignee | ||
Updated•11 years ago
|
Whiteboard: [c= p= s= u= ]
Updated•11 years ago
|
Whiteboard: [c= p= s= u= ]
Assignee | ||
Updated•11 years ago
|
Summary: seccomp-b2g support for ICS emulator → seccomp-b2g support for ICS emulator / on TBPL
Assignee | ||
Comment 1•11 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: b2g-seccomp
Assignee | ||
Comment 2•11 years ago
|
||
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•11 years ago
|
||
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•11 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•11 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•11 years ago
|
||
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•11 years ago
|
Attachment #8379976 -
Attachment is obsolete: true
Assignee | ||
Comment 8•11 years ago
|
||
Pull request for source. Carrying over r=kang.
Attachment #8380914 -
Flags: review+
Assignee | ||
Comment 9•11 years ago
|
||
Pull request for binaries. Carrying over r=kang.
Attachment #8380917 -
Flags: review+
Assignee | ||
Comment 10•11 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
Closed: 11 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 11•11 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•11 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•