Closed Bug 1650751 Opened 4 years ago Closed 4 years ago

Crash in [@ __fcntl64_nocancel_adjusted] (FMODE_NONOTIFY)

Categories

(Core :: Security: Process Sandboxing, defect, P1)

Unspecified
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla80
Tracking Status
firefox-esr68 --- wontfix
firefox-esr78 --- fixed
firefox78 --- wontfix
firefox79 --- fixed
firefox80 --- fixed

People

(Reporter: gsvelto, Assigned: jld)

References

(Regression)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(2 files)

This bug is for crash report bp-6a35a53f-6e3a-4529-ba85-33f9f0200703.

Top 10 frames of crashing thread:

0 libc.so.6 __fcntl64_nocancel_adjusted /build/glibc-YYA7BZ/glibc-2.31/io/../sysdeps/unix/sysv/linux/fcntl_nocancel.c:64
1 libc.so.6 __libc_fcntl64 /build/glibc-YYA7BZ/glibc-2.31/io/../sysdeps/unix/sysv/linux/fcntl64.c:51
2 libglib-2.0.so.0 <name omitted> ../../../glib/glib-unix.c:177
3 libgio-2.0.so.0 g_socket_constructed ./debian/build/deb/../../../gio/gsocket.c:709
4 libgobject-2.0.so.0 g_object_new_internal ../../../gobject/gobject.c:1977
5 libgobject-2.0.so.0 <name omitted> ../../../gobject/gobject.c:2262
6 libgio-2.0.so.0 g_initable_new_valist ./debian/build/deb/../../../gio/ginitable.c:244
7 libgio-2.0.so.0 g_initable_new ./debian/build/deb/../../../gio/ginitable.c:162
8 libgio-2.0.so.0 g_socket_new ./debian/build/deb/../../../gio/gsocket.c:1290
9 libgio-2.0.so.0 create_socket ./debian/build/deb/../../../gio/gsocketclient.c:136

This is happening in recent Linux distros, mostly Arch and Ubuntu 20.04. The crash starts in nsLookAndFeel::EnsureInit() which calls gtk_settings_get_for_display() and through a long and winding stack lands here:

https://github.com/GNOME/glib/blob/1ee22d0ae9145f0557fc9d1fc649ae22fd4c4f6f/gio/gsocket.c#L709

Which is implemented like so:

https://github.com/GNOME/glib/blob/mainline/glib/glib-unix.c#L177

None of this code is new, so I had to dig into glibc to figure out what's going and the chase led me to this change in glibc:

https://sourceware.org/git/?p=glibc.git;a=commit;h=bc2eb9321ec0d17d41596933617b2522c9aa5e0b

It's the only recent change that altered the syscall-calling code that appears in this stack. Unfortunately I have no clue what the change does :-|

The arguments for fcntl seem to be:

                    "rdi": "0x000000000000001e",
                    "rsi": "0x0000000000000004",
                    "rdx": "0x0000000004000802",

Seems we're calling fcntl(fd, F_SETFL, O_NONBLOCK|O_RDWR|0x4000000).
If I'm reading things right, 0x4000000 is FMODE_NONOTIFY which.. I'm not sure why it would appear here.

All of the reports appear to be linux v5.8-rc, could https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e9c15badbb7b20ccdbadf5da14e0a68fbad51015 explain this?

If I understand that commit message correctly, Linux sets this mode automatically on files that don't exist in the filesystem (like sockets, judging by comment #0). I assume it's returned in F_GETFL with the other flags (there doesn't seem to be any filtering), so user code trying to change a subset of flags passes it back to F_SETFL.

Linux's F_SETFL implementation has an allow list of flags it accepts (SETFL_MASK in the source) and ignores changes to anything else; our sandbox policy instead has a list of flags that are known to be ignored, because SETFL_MASK could expand in the future. And so we end up here.

This looks simple enough to fix.

Assignee: nobody → jld
Severity: -- → S3
Priority: -- → P1

As of kernel 5.8 (commit e9c15badb), Linux will set the internal
FMODE_NONOTIFY flag on files that don't exist in the filesystem,
including (unnamed) pipes and sockets. Although this flag isn't
properly part of the userspace API, it will be returned by F_GETFL, so
userspace code that tries to change file flags will pass it to F_SETFL.

The implementation of F_SETFL has an allow list of flags userspace can
change (SETFL_MASK) and ignores all others, but our sandbox has a list
of flags known to be ignored, because currently unknown flags could
potentially be accepted by the kernel in the future.

This patch adds FMODE_NONOTIFY as an ignored flag.

Crash Signature: [@ __fcntl64_nocancel_adjusted] → [@ __fcntl64_nocancel_adjusted] [@ __openat64_nocancel]
Pushed by jedavis@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9c0f809e9924 Add FMODE_NONOTIFY to ignored file flags in Linux sandbox. r=gcp
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80

Adjusting subject for contrast with the same crash signature with different cause in bug 1622728.

Crash Signature: [@ __fcntl64_nocancel_adjusted] [@ __openat64_nocancel] → [@ __fcntl64_nocancel_adjusted] [@ __openat64_nocancel]
Summary: Crash in [@ __fcntl64_nocancel_adjusted] → Crash in [@ __fcntl64_nocancel_adjusted] (FMODE_NONOTIFY)

We've had the F_SETFL filter for content processes since bug 1328896, and the socket process since when it was added; bug 1639181 just deduplicated those and moved it to the common policy (replacing the RDD process's previous policy, which was even more restrictive). So this will also affect ESR. I'll request uplifts.

Has Regression Range: --- → yes

Comment on attachment 9162954 [details]
Bug 1650751 - Add FMODE_NONOTIFY to ignored file flags in Linux sandbox.

Beta/Release Uplift Approval Request

  • User impact if declined: The bug is present only with Linux kernel 5.8, which is currently a release candidate. On Nightly we see crashes in WebRTC; on beta/release this won't crash but it will probably cause WebRTC not to work. There may be other impacted features not yet seen in crash reports.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The patch just allows some system calls that were previously denied (and which we know aren't a security risk), and the code content is simple (simply adds a flag to a set).
  • String changes made/needed: none
Attachment #9162954 - Flags: approval-mozilla-beta?

Comment on attachment 9162954 [details]
Bug 1650751 - Add FMODE_NONOTIFY to ignored file flags in Linux sandbox.

Approved for 79.0b9.

Attachment #9162954 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

As of kernel 5.8 (commit e9c15badb), Linux will set the internal
FMODE_NONOTIFY flag on files that don't exist in the filesystem,
including (unnamed) pipes and sockets. Although this flag isn't
properly part of the userspace API, it will be returned by F_GETFL, so
userspace code that tries to change file flags will pass it to F_SETFL.

The implementation of F_SETFL has an allow list of flags userspace can
change (SETFL_MASK) and ignores all others, but our sandbox has a list
of flags known to be ignored, because currently unknown flags could
potentially be accepted by the kernel in the future.

This patch adds FMODE_NONOTIFY as an ignored flag.

Attachment #9164215 - Attachment description: Bug 1650751 - Add FMODE_NONOTIFY to ignored file flags in Linux sandbox. → Bug 1650751 - Add FMODE_NONOTIFY to ignored file flags in Linux sandbox. (ESR78 backport)

Comment on attachment 9164215 [details]
Bug 1650751 - Add FMODE_NONOTIFY to ignored file flags in Linux sandbox.

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: WebRTC is likely to break when Linux kernel 5.8 is released and adopted by distributions. (Note that this is a backport of attachment 9162954 [details], due to the conflicting changes in bug 1639181.)
  • User impact if declined: The bug is present only with Linux kernel 5.8, which is currently a release candidate. On Nightly we see crashes in WebRTC; on beta/release this won't crash but it will probably cause WebRTC not to work. There may be other impacted features not yet seen in crash reports.
  • Fix Landed on Version: 79
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The patch just allows some system calls that were previously denied (and which we know aren't a security risk), and the code content is simple (simply adds a flag to a set).
  • String or UUID changes made by this patch: none
Attachment #9164215 - Flags: approval-mozilla-esr78?

Comment on attachment 9164215 [details]
Bug 1650751 - Add FMODE_NONOTIFY to ignored file flags in Linux sandbox.

Approved for 78.1esr.

Attachment #9164215 - Flags: approval-mozilla-esr78? → approval-mozilla-esr78+
Attachment #9164215 - Attachment description: Bug 1650751 - Add FMODE_NONOTIFY to ignored file flags in Linux sandbox. (ESR78 backport) → Bug 1650751 - Add FMODE_NONOTIFY to ignored file flags in Linux sandbox.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: