Crash in [@ __fxstatat]
Categories
(Core :: Security: Process Sandboxing, defect, P1)
Tracking
()
People
(Reporter: gsvelto, Assigned: jld)
References
Details
(Keywords: crash)
Crash Data
Attachments
(3 files)
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-esr78+
|
Details |
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-esr78+
|
Details |
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-esr78+
|
Details |
Crash report: https://crash-stats.mozilla.org/report/index/b6508761-63bc-471d-a88a-2bf7e0200821
Top 10 frames of crashing thread:
0 libc.so.6 __fxstatat /usr/src/debug/glibc-2.32/io/../sysdeps/unix/sysv/linux/wordsize-64/fxstatat.c:43
1 libc.so.6 statx_generic /usr/src/debug/glibc-2.32/io/./statx_generic.c:60
2 libgio-2.0.so.0 libgio-2.0.so.0@0x13ba9f
3 firefox-bin arena_t::MallocSmall memory/build/mozjemalloc.cpp:2864
4 libglib-2.0.so.0 libglib-2.0.so.0@0x5957c
5 libgobject-2.0.so.0 libgobject-2.0.so.0@0x19256
6 libgobject-2.0.so.0 libgobject-2.0.so.0@0x5ab8f
7 libgobject-2.0.so.0 libgobject-2.0.so.0@0x5ab8f
8 libgobject-2.0.so.0 libgobject-2.0.so.0@0x190af
9 libgobject-2.0.so.0 libgobject-2.0.so.0@0x5a33f
Found this during nightly crash triage and it prompted me to scrape symbols for the testing packages of Fedora 33. Unfortunately I don't have all the symbols yet as seen in the stack trace above but this seems caused by a new glibc calling newfsstatat()
(syscall 262).
Reporter | ||
Comment 1•4 years ago
|
||
I found a fully symbolicated crash and if you look at it you'll notice that I really need to fix bug 1648692 in order to have better traces on Fedora :-(
Assignee | ||
Comment 2•4 years ago
|
||
We support the case of fstatat
that's similar to stat
/lstat
, where dirfd == AT_FDCWD
. This is not that. But notice that the flags contain the Linux extension AT_EMPTY_PATH
, which allows passing an empty string as the path, for behavior like fstat
(including if dirfd
isn't a directory).
The path is at 0x00007fcd0ca3e177
, which is offset 0x16e177
in libgio-2.0.so.0
, so if the byte at that address is 0, then this is a case that could be supported (but currently isn't). The RPMs I've found didn't match the build ID we have, so I can't be sure, but this info (AT_EMPTY_PATH
and a constant path in libgio
) points to this glib commit, which is the fstat
-like case, as the likely cause.
Note that the path isn't required to be the empty string, so we can't pass this through in seccomp-bpf; it would have to be emulated with SECCOMP_RET_TRAP
. (Or, after bug 1603307, we'd need to extract a copy of the fd, call fstat
on it in the parent process, and write the struct stat
back into the child's address space.)
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 3•4 years ago
|
||
Assignee | ||
Comment 4•4 years ago
|
||
Comment 6•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/80baa04419c4
https://hg.mozilla.org/mozilla-central/rev/56f6da03c7e3
Comment 7•4 years ago
|
||
Is this something we should consider backporting eventually or can it ride the trains to release?
Comment 8•4 years ago
|
||
Looks like we're still seeing crashes e.g. bp-315269cb-bb57-4158-b9be-3b89c0200902 is on nightly 20200901094542 which should have had these patches.
Comment 9•4 years ago
|
||
AFAICT in this case it's calling g_local_file_lstat
which doesn't use AT_EMPTY_PATH
Comment 10•4 years ago
|
||
Per the manpage "Both stat() and lstat() act as though AT_NO_AUTOMOUNT
was set.", so don't bail if it's set in a call to fstatat.
Comment 11•4 years ago
|
||
Comment 12•4 years ago
|
||
bugherder |
Comment 13•4 years ago
|
||
AFAICT, this only crashes on Nightly builds and doesn't need uplifting. Feel free to nominate approval if I'm misunderstanding something, though.
Comment 14•4 years ago
|
||
I don't know how glib reacts to statx returning an error, we may want to uplift to esr78 to avoid finding out when glib 2.66 is released?
Updated•4 years ago
|
Updated•4 years ago
|
Comment 15•4 years ago
|
||
glib 2.66 is out and it hits Firefox 80.0.1 with many warnings
Sandbox: seccomp sandbox violation: pid 301342, tid 301342, syscall 262, args 33 140385171460495 140731898391248 4096 4096 1.
Sandbox: unsupported fd-relative fstatat(33, "", 0x7FFEB2CFEAD0, 4096)
The PID is from a content process, and none of content processes crashes due to this error. I feel it annoying, so I rebuild Firefox with changesets 80baa04419c4 and b1d7a6be184b, and there are no such warnings anymore.
I'm using Firefox 80.0.1 on Arch Linux.
There is another report with similar warnings from ESR 78.2.0 on Debian sid [1]. I guess it is also related to glib 2.66.
[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=970196
Assignee | ||
Comment 16•4 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #7)
Is this something we should consider backporting eventually or can it ride the trains to release?
We should uplift these patches: we may not immediately crash on release, but we're still failing the syscall and the caller probably won't have a fallback (these flags have existed since Linux 2.6.39), and this could cause problems.
The fstat
case seen in the original reports is probably harmless — it's checking if a file is a directory when preparing to read from it, and it fails open which in context (GTK stylesheets?) is probably safe. But other libraries could also try to use it. And the case from comment #8 fixed in the followup patch is worse: that's lstat
-equivalent, and from the call stack it looks like we could fail to load GTK theme / UI settings information as a result.
I'll write an uplift request.
Assignee | ||
Comment 17•4 years ago
|
||
Comment on attachment 9172528 [details]
Bug 1660901 - Support the fstat-like subset of fstatat in the Linux sandbox policies.
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration: These patches fix the sandbox policy to handle changes in a new version of glib, which Linux distributions are beginning to ship (alongside older Firefox).
- User impact if declined: This can cause anything using glib, in particular GTK, to see files as not existing (D89121), or to fail to correctly determine information about opened files (D88499). In the case of GTK this can affect reading theme and UI settings information. There may be other impacts we're not aware of yet.
- Fix Landed on Version: 82
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): These patches add handling for system calls which were previously denied, and don't affect cases that were previously allowed. (More specifically: D89121 simply ignores setting a flag that we previously de facto treated as set, and D88499 is a simple rearrangement of function arguments to substitute a syscall that we do allow. There's relatively little subtlety in these patches.) D88500 has simple regression tests, although they aren't exhaustive.
- String or UUID changes made by this patch: none
Assignee | ||
Updated•4 years ago
|
Comment 18•4 years ago
|
||
Comment on attachment 9172528 [details]
Bug 1660901 - Support the fstat-like subset of fstatat in the Linux sandbox policies.
Approved for 78.4esr.
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Comment 19•4 years ago
|
||
bugherder uplift |
Comment 21•4 years ago
|
||
Any build before this landed is now crashing when bisecting. Any tips how to run mozregression now?
I'm on Manjaro KDE.
Comment 22•4 years ago
|
||
--pref "security.sandbox.content.level:0"
worked for me.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Updated•4 years ago
|
Description
•