1134942 - Whitelist fstatat and unlinkat for B2G content processes

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

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

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

Comment 1

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

Attachment: bug1134942-whitelist-lollipop-jars-hg0.diff

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.

Comment 2

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

Comment on attachment 8567266 [details] [diff] [review]
bug1134942-whitelist-lollipop-jars-hg0.diff

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

how soon is soon ? :)

Comment 3

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

(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

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/128980c4abde

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/128980c4abde

Comment 6

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

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.

Comment 7

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Hi Steven,
Do we really need it for L porting?

Comment 8

[Kai-Zhen Li [:seinlin][:kli]]

(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!

Comment 9

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Spoke with Shawn and yes we'll need it for v2.2.

Comment 10

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

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.

Comment 11

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

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

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/1b262015bb54