1673202 - [Fedora 34] Startup Crash in [@ mozilla::SandboxPolicyCommon::StatAtTrap] and [@ mozilla::SigSysHandler]

Status: RESOLVED FIXED

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

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Crash reports for at least 5 installations, all running Fedora 34

Crash report: https://crash-stats.mozilla.org/report/index/2b5823c7-77dd-464e-87fe-b6abe0201022

Reason: SIGSEGV /SEGV_MAPERR

Top 1 frames of crashing thread:

0 libmozsandbox.so mozilla::SandboxPolicyCommon::StatAtTrap /builddir/build/BUILD/firefox-82.0/security/sandbox/linux/SandboxFilter.cpp:240

Comment 1

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

There are also [@ mozilla::SigSysHandler] crashes.

Comment 2

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

This is bad. The crashes all seem to be on writes to the stack.

I think this is infinite recursion: we catch fstatat, recognize the AT_EMPTY_PATH flag and empty path, and map it to a call to fstat… and then libc maps it back to fstatat.

This was added in glibc 2.33, judging by the symbol versions:

00000000000f1730 W fstat@@GLIBC_2.33
00000000000f1730 W fstat64@@GLIBC_2.33

(gdb) disas fstat
Dump of assembler code for function fstat64:
   0x00000000000f1730 <+0>:     endbr64 
   0x00000000000f1734 <+4>:     mov    %rsi,%rdx
   0x00000000000f1737 <+7>:     mov    $0x1000,%ecx
   0x00000000000f173c <+12>:    lea    0x9dddf(%rip),%rsi        # 0x18f522
   0x00000000000f1743 <+19>:    jmpq   0xf1770 <fstatat64>

Older glibc has fstat as an inline function that expands into a call to __fxstat, which (despite the name suggesting statx) is actually fstat with an ABI version number(?).

This is fixable: invoke the fstat syscall directly.

But it's not great that glibc does that; this case can't be handled in seccomp-bpf itself (because it depends on the memory at the path pointer), so every call to fstat is going to have to be processed in userland code. Currently that means a signal handler; post-USERNOTIF, that means context switching to another process, extracting the fd from the child, fstating it in the other process, writing the result back into the child's address space, and then finally unblocking the child. This is in contrast to the real fstat, which can just be allowed in the seccomp filter (a relatively fast path that the kernel developers are trying to make faster).

I suppose if that becomes a performance issue, we could interpose fstat to call the real fstat.

Comment 3

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

Fedora's bug report has a proper stack trace (ours cuts off after the first frame), clearly showing the unbounded recursion.

Comment 4

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

The glibc change that caused this seems to be 8ed005daf.

As noticed on the Fedora bug, this affects only their builds (and only Rawhide, not release Fedora; glibc 2.33 isn't released yet), and not Mozilla's builds, because it depends on whether those inline function definitions are present in the headers on the build host.

Comment 5

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

Another problem: GMP processes have no broker, so they don't get the fstatat trap, so fstat will fail with ENOSYS (or, on Nightly, crash with SIGSYS). That's less severe; I'll file a followup bug.

Comment 6

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

Attachment: Bug 1673202 - Call fstat directly in Linux sandbox fstatat interception.

Sandbox policies handle the case of fstatat(fd, "", AT_EMPTY_PATH|...)
by invoking the SIGSYS handler (because seccomp-bpf can't tell if the
string will be empty when the syscall would use it), which makes the
equivalent call to fstat.

Unfortunately, recent development versions of glibc implement fstat by
calling fstatat, which causes unbounded recursion and stack overflow.
(This depends on the headers present at build time; see the bug for more
details.) This patch switches it to use the fstat (or fstat64 on
32-bit) syscall directly.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/63d29101d708
Call fstat directly in Linux sandbox fstatat interception. r=gcp

Comment 8

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/63d29101d708

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jld, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 11

[Pascal Chevrel:pascalc]

Jed, our last beta builds tomorrow, should we uplift this?

Comment 12

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

(In reply to Pascal Chevrel:pascalc from comment #11)

Jed, our last beta builds tomorrow, should we uplift this?

I don't think we need to; see comment #4. It doesn't affect our builds, only downstream builds using a prerelease version of glibc on the build host, like Fedora Rawhide — and Fedora already took the patch.

But we'll likely want to uplift to ESR 78.

Comment 13

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 9183888 [details]
Bug 1673202 - Call fstat directly in Linux sandbox fstatat interception.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Without this patch Firefox 78 ESR might stop working on to-be-released Fedora distributions. (Our own builds would still work, but not distro shipped ones)
• User impact if declined: Nonfunctional ESR 78 in the future.
• Fix Landed on Version: 84
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It is risky in the sense that Firefox 84 hasn't rolled out to release yet and we'd likely want to do that before the uplift.

(No distro right now will hit this case)

• String or UUID changes made by this patch:

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9183888 [details]
Bug 1673202 - Call fstat directly in Linux sandbox fstatat interception.

Approved for 78.6esr.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/13d5867b0399