Closed Bug 1134942 Opened 5 years ago Closed 5 years ago

Whitelist fstatat and unlinkat for B2G content processes

Categories

(Core :: Security: Process Sandboxing, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
blocking-b2g 2.2+
Tracking Status
firefox37 --- wontfix
firefox38 --- wontfix
firefox39 --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: jld, Assigned: jld)

References

Details

Attachments

(1 file)

Until bug 1034143 is fixed, tests that use jar:http: URLs (and we have some) will crash — as will actual Web content that does so, if any exists — because the patch that updated the seccomp-bpf whitelist to account for [the Android L system call changes][f8fcfbc] didn't include stat/lstat and unlink, probably because (almost?) nothing else in B2G still tries to use them while sandboxed.

The fix is simple, and might as well be committed now so it doesn't block people later.

[f8fcfbc]: https://android.googlesource.com/platform/bionic.git/+/f8fcfbc85a3ce3e195626b90736d3a484331494b
This is not the direction I'd like to be going with the whitelist, obviously, but it's not really adding anything that isn't already there, and it's all going to be removed relatively soon.
Attachment #8567266 - Flags: review?(gdestuynder)
Comment on attachment 8567266 [details] [diff] [review]
bug1134942-whitelist-lollipop-jars-hg0.diff

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

how soon is soon ? :)
Attachment #8567266 - Flags: review?(gdestuynder) → review+
(In reply to Guillaume Destuynder [:kang] from comment #2)
> how soon is soon ? :)

That depends on how particular the Necko peers are, but hopefully within the next week or so.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca6f20c77731
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/128980c4abde
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
If we're supporting 2.2 on Lollipop — and, judging by other bugs, we are — we'll want to uplift this patch.  It just adds system calls to the whitelist, so they'll succeed in cases where they would otherwise have caused a crash; therefore there's effectively no risk.

(In reply to Jed Davis [:jld] from comment #0)

> Until bug 1034143 is fixed, tests that use jar:http: URLs (and we have some)
> will crash

...and possibly more than that; jar:http: was the only remaining user of unlinkat, but I think there might be other users of fstatat.
blocking-b2g: --- → 2.2?
Hi Steven,
Do we really need it for L porting?
Flags: needinfo?(styang)
(In reply to Jed Davis [:jld] from comment #6)
> If we're supporting 2.2 on Lollipop — and, judging by other bugs, we are —
> we'll want to uplift this patch.  It just adds system calls to the
> whitelist, so they'll succeed in cases where they would otherwise have
> caused a crash; therefore there's effectively no risk.

Jed, 2.2 support Lollipop, so I think we uplift this patch to 2.2. Could you request for approval‑mozilla‑b2g37? Thanks!
Flags: needinfo?(jld)
Spoke with Shawn and yes we'll need it for v2.2.
blocking-b2g: 2.2? → 2.2+
Flags: needinfo?(styang)
Comment on attachment 8567266 [details] [diff] [review]
bug1134942-whitelist-lollipop-jars-hg0.diff

NOTE: please apply this patch before the one from bug 1140111 to avoid unnecessary merge conflicts.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 1094121
User impact if declined: App crashes and test failures on B2G Lollipop.
Testing completed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f3a4230cd05 
Risk to taking this patch (and alternatives if risky): None; it just causes system calls to succeed that would previously result in a crash.
String or UUID changes made by this patch: None.
Flags: needinfo?(jld)
Attachment #8567266 - Flags: approval-mozilla-b2g37?
(In reply to Jed Davis [:jld] from comment #10)
> Testing completed:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f3a4230cd05 

Also, built locally for nexus-5-l (v2.2 branch) and verified it prevents crashing on web content that does jar:https:// XHR.
Attachment #8567266 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Duplicate of this bug: 1144267
You need to log in before you can comment on or make changes to this bug.