1715254 - Sandbox doesn't work with clone3

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[H.J. Lu]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.77 Safari/537.36

Steps to reproduce:

I am adding clone3 support to glibc 2.34:

https://sourceware.org/pipermail/libc-alpha/2021-May/126841.html

The clone3 system call provides a superset of the functionality of clone
and clone2. It also provides a number of API improvements, including
the ability to specify the size of the child's stack area which can be
used by kernel to compute the shadow stack size when allocating the
shadow stack. Add:

extern int __clone_internal (struct clone_args __cl_args,
int (__func) (void *__arg), void *__arg);

to provide an abstract interface for clone, clone2 and clone3.

1. Simplify stack management for thread creation by passing both stack
   base and size to create_thread.
2. Consolidate clone vs clone2 differences into a single file.
3. Call __clone3 if HAVE_CLONE3_WAPPER is defined. If __clone3 returns
   -1 with ENOSYS, fall back to clone or clone2.
4. Use only __clone_internal to clone a thread. Since the stack size
   argument for create_thread is now unconditional, always pass stack size
   to create_thread.
5. Enable the public clone3 wrapper in the future after it has been
   added to all targets.

Please add clone3 support to sandbox so that it will work with glibc 2.34.

Actual results:

Firebox crashes with glibc 2.34.

Expected results:

Firefox should work with glibc 2.34.

Comment 1

[H.J. Lu]

The same issue has been fixed in chromium by:

The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/5e08782516d24de536e75d6bf4ff2bc87be55124

commit 5e08782516d24de536e75d6bf4ff2bc87be55124
Author: Matthew Denton <[e-mail]>
Date: Thu Jun 03 19:02:10 2021

Linux sandbox: update syscall numbers for all platforms.

This includes clone3 and the landlock system calls.

Bug: 1213452
Change-Id: Iaf14a7c9d455c7a22ad179b13541a60dcabaac09
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2934620
Auto-Submit: Matthew Denton <[e-mail]>
Commit-Queue: Robert Sesek <[e-mail]>
Reviewed-by: Robert Sesek <[e-mail]>
Cr-Commit-Position: refs/heads/master@{#888958}

[modify] https://crrev.com/5e08782516d24de536e75d6bf4ff2bc87be55124/sandbox/linux/system_headers/arm64_linux_syscalls.h
[modify] https://crrev.com/5e08782516d24de536e75d6bf4ff2bc87be55124/sandbox/linux/system_headers/arm_linux_syscalls.h
[modify] https://crrev.com/5e08782516d24de536e75d6bf4ff2bc87be55124/sandbox/linux/system_headers/mips64_linux_syscalls.h
[modify] https://crrev.com/5e08782516d24de536e75d6bf4ff2bc87be55124/sandbox/linux/system_headers/mips_linux_syscalls.h
[modify] https://crrev.com/5e08782516d24de536e75d6bf4ff2bc87be55124/sandbox/linux/system_headers/x86_32_linux_syscalls.h
[modify] https://crrev.com/5e08782516d24de536e75d6bf4ff2bc87be55124/sandbox/linux/system_headers/x86_64_linux_syscalls.h

Comment 2

[H.J. Lu]

Also

The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/218438259dd795456f0a48f67cbe5b4e520db88b

commit 218438259dd795456f0a48f67cbe5b4e520db88b
Author: Matthew Denton <[e-mail]>
Date: Thu Jun 03 20:06:13 2021

Linux sandbox: return ENOSYS for clone3

Because clone3 uses a pointer argument rather than a flags argument, we
cannot examine the contents with seccomp, which is essential to
preventing sandboxed processes from starting other processes. So, we
won't be able to support clone3 in Chromium. This CL modifies the
BPF policy to return ENOSYS for clone3 so glibc always uses the fallback
to clone.

Bug: 1213452
Change-Id: I7c7c585a319e0264eac5b1ebee1a45be2d782303
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2936184
Reviewed-by: Robert Sesek <[e-mail]>
Commit-Queue: Matthew Denton <[e-mail]>
Cr-Commit-Position: refs/heads/master@{#888980}

[modify] https://crrev.com/218438259dd795456f0a48f67cbe5b4e520db88b/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc

Comment 3

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

The Bugbug bot thinks this bug should belong to the 'Core::Security: Process Sandboxing' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 4

[:gerard-majax]

Thanks for filing that ahead so we can support it properly when it lands.

Looking at the email threads, it looks like we will have to adopt the same solution as Chromium did, but I fail to find the actual patch merged in current glibc repo, where is the implementation?

Comment 5

[:gerard-majax]

Attachment: Bug 1715254 - Deny clone3 to force glibc fallback r?gcp

Comment 6

[Florian Weimer]

It's not merged yet. We are sort-of blocked on Firefox and Chromium allowing clone3. Ideally, distributions using glibc snapshots would have something to backport for the browsers.

Comment 7

[:gerard-majax]

(In reply to Florian Weimer from comment #6)

It's not merged yet. We are sort-of blocked on Firefox and Chromium allowing clone3. Ideally, distributions using glibc snapshots would have something to backport for the browsers.

Thanks, the wording on the bug report made it sound like it was already merged and this was breaking some distro using newer/dev version of glibc.

Comment 8

[Gian-Carlo Pascutto [:gcp]]

One key question here: We allow clone() in the sandbox for thread creation: https://searchfox.org/mozilla-central/source/security/sandbox/linux/SandboxFilter.cpp#858

This is done by inspecting the arguments. Can the arguments for clone3() be inspected safely from a seccomp-bpf filter? As far as I know, when they were first proposed, the conclusion was that they were unsandboxable.

https://lwn.net/Articles/792628/
https://lore.kernel.org/linux-api/20191028172143.4vnnjpdljfnexaq5@wittgenstein/

It's possible the design has changed since, I haven't looked.

If they can't, glibc will have to support the fallback paths for a loooong time.

Edit: After reading more, I noticed the Chromium commit message says as much too:
"So, we won't be able to support clone3 in Chromium. "

Comment 9

[Florian Weimer]

If glibc gets ENOSYS, that will be fine for now.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ecb4011a0c76
Deny clone3 to force glibc fallback r=gcp

Comment 11

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

I don't think there's anything that we (or Chromium) can do here other than ENOSYS, until/unless seccomp-bpf gains a framework for safely inspecting these extended-arguments structs. IIRC there was a proposal for that at one point (specifically these structs, not memory arguments in general), but ran into design problems around how to handle the struct being larger/newer than what the filter understands — the filter can't loop over the remainder to verify if it's zeroed because it can't loop, so it would need a way to tell the kernel to do that alongside the return value, and BPF does have a store instruction but I don't think it's hooked up to anything currently, etc.

For the sake of completeness, it would be possible to use SECCOMP_RET_TRAP to substitute a clone call if possible, but only if SIGSYS isn't blocked; I forget if glibc already blocks signals during thread creation, but it seems like the kind of operation where there's risk of that needing to happen in the future.

Note that SECCOMP_RET_USER_NOTIF doesn't help: as far as I know there's no Windows-like thread injection that a supervisor process could use.

Comment 12

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/ecb4011a0c76