Remove unlink() from seccomp-bpf whitelist

RESOLVED FIXED in Firefox 39

Status

()

RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jld, Assigned: jld)

Tracking

Trunk
mozilla39
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox39 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
STR: start b2g Messages app; poke the button to compose a new message.  Crash is immediate and 100% reproducible, for me.  This is on keon, but I don't think it matters.

I'm assuming that seccomp is being reasonable in disallowing unlink, so I've filed this bug with where the [[Modules/Core]] wiki page maps the files that look sort of responsible.  Adjust categorization as needed.

(gdb) bt
#0  unlink () at bionic/libc/arch-arm/syscalls/unlink.S:10
#1  0x420087aa in PR_Delete (name=0x43bde8b8 "/data/local/tmp/mozilla-temp-561545566")
    at /home/jld/src/B2G/gecko/nsprpub/pr/src/pthreads/ptio.c:3598
#2  0x4108928e in nsLocalFile::OpenNSPRFileDesc (this=0x442dc430, flags=-2147483644, 
    mode=<value optimized out>, _retval=0xbec8dbd4)
    at /home/jld/src/B2G/gecko/xpcom/io/nsLocalFileUnix.cpp:390
#3  0x4108aa16 in NS_OpenAnonymousTemporaryFile (aOutFileDesc=0xbec8dbd4)
    at /home/jld/src/B2G/gecko/xpcom/io/nsAnonymousTemporaryFile.cpp:99
#4  0x40a66d94 in mozilla::MediaCache::Init (this=0x446c8510)
    at /home/jld/src/B2G/gecko/content/media/MediaCache.cpp:536
#5  0x40a66e56 in InitMediaCache (this=0x44508088)
    at /home/jld/src/B2G/gecko/content/media/MediaCache.cpp:612
#6  mozilla::MediaCacheStream::Init (this=0x44508088)
    at /home/jld/src/B2G/gecko/content/media/MediaCache.cpp:2288
#7  0x40a72016 in mozilla::ChannelMediaResource::Open (this=0x44508000, aStreamListener=0x446a5638)
    at /home/jld/src/B2G/gecko/content/media/MediaResource.cpp:593
#8  0x40a68d90 in mozilla::MediaDecoder::OpenResource (this=0x442077a0, aStreamListener=0x446a5638)
    at /home/jld/src/B2G/gecko/content/media/MediaDecoder.cpp:462
#9  0x40a69dd6 in mozilla::MediaDecoder::Load (this=0x10, aStreamListener=0x1, aCloneDonor=0x1)
    at /home/jld/src/B2G/gecko/content/media/MediaDecoder.cpp:476
#10 0x40a2cb02 in mozilla::dom::HTMLMediaElement::FinishDecoderSetup (this=0x4369fc10, 
    aDecoder=0x442077a0, aStream=<value optimized out>, aListener=0x446a5638, aCloneDonor=0x0)
    at /home/jld/src/B2G/gecko/content/html/content/src/HTMLMediaElement.cpp:2546
#11 0x40a2d69a in mozilla::dom::HTMLMediaElement::InitializeDecoderForChannel (this=0x4369fc10, 
    aChannel=<value optimized out>, aListener=0x446a5638)
    at /home/jld/src/B2G/gecko/content/html/content/src/HTMLMediaElement.cpp:2507
#12 0x40a2e72a in mozilla::dom::HTMLMediaElement::MediaLoadListener::OnStartRequest (
    this=0x446a5620, aRequest=0x43ef4ac0, aContext=0x0)
    at /home/jld/src/B2G/gecko/content/html/content/src/HTMLMediaElement.cpp:338
#13 0x407f589e in nsJARChannel::OnStartRequest (this=<value optimized out>, 
    req=<value optimized out>, ctx=<value optimized out>)
    at /home/jld/src/B2G/gecko/modules/libjar/nsJARChannel.cpp:969
#14 0x4071f0de in nsInputStreamPump::OnStateStart (this=0x43fae520)
    at /home/jld/src/B2G/gecko/netwerk/base/src/nsInputStreamPump.cpp:463
#15 0x4071f3ba in nsInputStreamPump::OnInputStreamReady (this=0x43fae520, 
    stream=<value optimized out>)
    at /home/jld/src/B2G/gecko/netwerk/base/src/nsInputStreamPump.cpp:388
#16 0x41090c1a in nsInputStreamReadyEvent::Run (this=0x446a5660)
    at /home/jld/src/B2G/gecko/xpcom/io/nsStreamUtils.cpp:160
#17 0x41099d10 in nsThread::ProcessNextEvent (this=0x40316940, mayWait=<value optimized out>, 
    result=0xbec8dff7) at /home/jld/src/B2G/gecko/xpcom/threads/nsThread.cpp:622
#18 0x41079778 in NS_ProcessNextEvent (thread=0x43fae520, mayWait=false)
    at /home/jld/src/B2G/objdir-gecko/xpcom/build/nsThreadUtils.cpp:238
#19 0x40e1ae4c in mozilla::ipc::MessagePump::Run (this=0x40301b80, aDelegate=0xbec8e904)
    at /home/jld/src/B2G/gecko/ipc/glue/MessagePump.cpp:81
#20 0x40e1aefe in mozilla::ipc::MessagePumpForChildProcess::Run (this=0x40301b80, 
    aDelegate=0xbec8e904) at /home/jld/src/B2G/gecko/ipc/glue/MessagePump.cpp:234
#21 0x410b6194 in MessageLoop::RunInternal (this=0x1000000)
    at /home/jld/src/B2G/gecko/ipc/chromium/src/base/message_loop.cc:220
#22 0x410b6212 in MessageLoop::RunHandler (this=0xbec8e904)
    at /home/jld/src/B2G/gecko/ipc/chromium/src/base/message_loop.cc:213
#23 MessageLoop::Run (this=0xbec8e904)
    at /home/jld/src/B2G/gecko/ipc/chromium/src/base/message_loop.cc:187
#24 0x40dcf630 in nsBaseAppShell::Run (this=0x4368de80)
    at /home/jld/src/B2G/gecko/widget/xpwidgets/nsBaseAppShell.cpp:161
#25 0x406c461a in XRE_RunAppShell ()
    at /home/jld/src/B2G/gecko/toolkit/xre/nsEmbedFunctions.cpp:679
#26 0x40e1aecc in mozilla::ipc::MessagePumpForChildProcess::Run (this=0x40301b80, 
    aDelegate=0xbec8e904) at /home/jld/src/B2G/gecko/ipc/glue/MessagePump.cpp:201
#27 0x410b6194 in MessageLoop::RunInternal (this=0x4368de80)
    at /home/jld/src/B2G/gecko/ipc/chromium/src/base/message_loop.cc:220
#28 0x410b6212 in MessageLoop::RunHandler (this=0xbec8e904)
    at /home/jld/src/B2G/gecko/ipc/chromium/src/base/message_loop.cc:213
#29 MessageLoop::Run (this=0xbec8e904)
    at /home/jld/src/B2G/gecko/ipc/chromium/src/base/message_loop.cc:187
#30 0x406c4b0e in XRE_InitChildProcess (aArgc=-1094129004, aArgv=0xbec8ea0c, aProcess=1077134336)
    at /home/jld/src/B2G/gecko/toolkit/xre/nsEmbedFunctions.cpp:516
#31 0x000086a0 in main (argc=7, argv=0xbec8ea94)
    at /home/jld/src/B2G/gecko/ipc/app/MozillaRuntimeMain.cpp:85
Component: DOM → Video/Audio
So, this doesn't happen on Buri.

It happens on Keon 100%.

I tried with the same gecko in both, but the cause is probably deeper.

Is there something we can try in the Messages app to workaround this ? I don't understand exactly what happens here :)
Ok, I've cat /proc/<N>/status, and on buri there is no "seccomp" line.
(Assignee)

Comment 3

5 years ago
Right.  Currently seccomp is only keon and peak, because :kang had to backport the seccomp-bpf kernel patches, and those are the only targets where the kernel is built from source as part of the B2G/Gonk build.  Some of the TARGET_NO_KERNEL devices might be fixable, if they can use a documented protocol to flash the kernel image, but I don't know if buri/hamachi is one of them.
How could we solve this correctly in your opinion?

Why is this happening only in the Messages app?
Blocks: 907087
(In reply to Julien Wajsberg [:julienw] from comment #4)
> How could we solve this correctly in your opinion?

We either need to move the file I/O for the media cache to the parent process, or we need to use argument filtering to ensure that we allow unlinking only of files in the media cache but disallow unlinking of other files.
(Assignee)

Comment 6

5 years ago
(In reply to Brian Smith (:briansmith, was :bsmith@mozilla.com) from comment #5)
> We either need to move the file I/O for the media cache to the parent
> process, or we need to use argument filtering to ensure that we allow
> unlinking only of files in the media cache but disallow unlinking of other
> files.

seccomp-bpf can't inspect the path passed to unlink.  There's no facility for reading the process's memory, and it would be a time-of-check/time-of-use issue if there were one.  The closest thing we could do, as far as I'm aware, is intercept the library call in libmozglue and remote it to the parent process.
Looks like it's trying to open an anonymous, temporary file. Can NS_OpenAnonymousTemporaryFile remote to the parent?
The anonymous temporary file is for caching downloaded media data. We need that to play video and audio. The file is unlinked since we don't need the media cache to be persistent.

The media cache usage could be remoted to a single media cache in the parent process. We discussed it in bug 785662 but Roc was opposed to it at that time. The advantage would be that we'd not need a separate media cache per process, so we'd reduce disk usage by the media cache.

I'm not sure if anyone from the media team is free to work on this.
The media cache on B2G is tiny, only 4MB. Using a shared cache would be nice in theory but unnecessary complexity in practice.

I assume our seccomp-bpf filters let us call close()? In that case, we could extend NS_OpenAnonymousTemporaryFile so that in a content process, it makes an IPDL call to the parent process to open and unlink the file, passing the file descriptor back to the content process.
(Assignee)

Updated

5 years ago
Blocks: 930258
(Assignee)

Updated

5 years ago
Depends on: 940863
Depends on: 965724
(Assignee)

Comment 10

4 years ago
I think MediaCache was the only unlink()er; I'm currently testing whether that's correct.
Assignee: nobody → jld
(Assignee)

Comment 11

4 years ago
(In reply to Jed Davis [:jld] from comment #10)
> I think MediaCache was the only unlink()er; I'm currently testing whether
> that's correct.

Nope: https://tbpl.mozilla.org/?tree=Try&rev=911fe7807b54
Summary: b2g Messages app crashes because MediaCache tries to call unlink() under seccomp → Remove unlink() from seccomp-bpf whitelist
(Assignee)

Updated

4 years ago
Depends on: 1031583
(Assignee)

Updated

4 years ago
Depends on: 1034143
(Assignee)

Updated

4 years ago
Depends on: 1058977
(Assignee)

Comment 12

4 years ago
Created attachment 8576193 [details] [diff] [review]
bug906996-finally-hg0.diff

It took a while, but it looks like we finally got there.

This try push is a little confusing because it was on top of inbound: https://treeherder.mozilla.org/#/jobs?repo=try&revision=db922dce686a
Attachment #8576193 - Flags: review?(gdestuynder)
(Assignee)

Updated

4 years ago
Component: Video/Audio → Security: Process Sandboxing
Comment on attachment 8576193 [details] [diff] [review]
bug906996-finally-hg0.diff

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

woohoo!
Attachment #8576193 - Flags: review?(gdestuynder) → review+
(Assignee)

Comment 14

4 years ago
Try run linked in comment #12; it should be correctly starred now.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/35ad2e5b036b
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.