1650751 - Crash in [@ __fcntl64_nocancel_adjusted] (FMODE_NONOTIFY)

Status: RESOLVED FIXED

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

Description

[Gabriele Svelto [:gsvelto]]

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 :-|

Comment 1

[Julien Cristau [:jcristau] (back April 22)]

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?

Comment 2

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

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.

Comment 3

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

This looks simple enough to fix.

Comment 4

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

Attachment: Bug 1650751 - Add FMODE_NONOTIFY to ignored file flags in Linux sandbox.

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.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c0f809e9924
Add FMODE_NONOTIFY to ignored file flags in Linux sandbox. r=gcp

Comment 7

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/9c0f809e9924

Comment 8

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

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

Comment 9

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

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.

Comment 10

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

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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

Approved for 79.0b9.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d3390cdad4fa

Comment 13

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

Attachment: Bug 1650751 - Add FMODE_NONOTIFY to ignored file flags in Linux sandbox.

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.

Comment 14

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

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

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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

Approved for 78.1esr.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/435eb575e449