Closed Bug 1670277 Opened 4 years ago Closed 4 years ago

Geckoview example crashes immediately on startup

Categories

(Core :: IPC, defect)

defect

Tracking

()

RESOLVED FIXED
84 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox82 --- unaffected
firefox83 --- unaffected
firefox84 --- fixed

People

(Reporter: jnicol, Assigned: jld)

References

(Regression)

Details

(Keywords: regression)

This is a regression from bug 1440203. GVE crashes immediately on startup, on both my Pixel 2 (Android 11) and Samsung S9 (Android 10).

I see this error my logcat:

WARNING: failed to create read-only memfd: Permission denied: file /home/jamie/src/gecko/ipc/chromium/src/base/shared_memory_posix.cc:245
Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0 in tid 23100 (Gecko), pid 23073 (ckoview_example)

Flags: needinfo?(jld)

Shouldn't we be using ashm on Android instead of memfd?

We'll use memfd in preference to ashmem if they're both available, which seemed reasonable enough, and Android itself is planning to migrate to memfd in the future. However, the memfd backend uses procfs to create read-only fds to pass to child processes, and that's what's being denied there. But also, there's a check for whether /proc/self/fd exists (because Tor Browser is interested in running desktop Firefox without /proc to minimize privacy exposure), and that would have to have succeeded.

There's also a question of whether the read-only thing is even meaningful on Android, since we don't have a sandbox there, so it may not be accomplishing anything, unless the OS has very carefully locked down a number of debugging APIs. Also, with kernel ≥5.1 we could rely on F_SEAL_FUTURE_WRITE (contributed to Linux by Android as part of migrating away from ashmem) for security instead of procfs.

Right now, I'm going to request backout of bug 1440203 until I know what's going on.

Assignee: nobody → jld
Flags: needinfo?(jld)

Conveniently I have a Pixel 2, and I can reproduce this (using python ctypes in termux) and see this in logcat:

10-12 10:48:35.807 8798 8798 W python : type=1400 audit(0.0:65852): avc: denied { open } for path=2F6D656D66643A74202864656C6574656429 dev="tmpfs" ino=12598758 scontext=u:r:untrusted_app_27:s0:c199,c256,c512,c768 tcontext=u:object_r:appdomain_tmpfs:s0:c199,c256,c512,c768 tclass=file permissive=0 app=com.termux

It doesn't reproduce for a normal unlinked file on ext4. I'm going to try to find the source for the SELinux policy to at least see if there's a rationale for this or some other context we should know about.

As far as I can tell, this hasn't worked since Android 5.0 turned on SELinux enforcement: when an app creates a tmpfs file (including memfd and ashmem, which use tmpfs internally), the policy has always assigned it a label for which the apps don't have open permission. (For reference, this is from the app_domain macro in public/te_macros in platform/system/sepolicy.) And, as we can see from the error message in comment #3, re-opening an anonymous inode via /proc/<pid>/fd counts as an open.

(Which raises the question of why it didn't fail in CI, which in theory is running Android 7; maybe the kernel is older than 3.17 and doesn't have memfd_create.)

Resolution: Don't use memfd on Android when relanding bug 1440203. For context, originally I thought it would be an advantage over ashmem, due to not understanding how ashmem permission changes work (which isn't well documented) and not yet realizing that memfd's F_SEAL_WRITE effectively isn't usable for us (see bug 1440203 comment #10); however, as I now understand the situation, there's no real reason not to just use ashmem.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.