1438394 - The fglrx detection from bug 1376910 is sometimes not working.

Status: RESOLVED FIXED

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

Description

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

I'm seeing content processes being prevented from calling semget(), with fglrx drivers in the stack… and GPU vendor ID equal to "ATI Technologies Inc.", which means the patch in bug 1376910 *should* be allowing access to SysV IPC, including semaphores.  I don't know what's going on here yet.

Comment 1

[Andrew McCreight [:mccr8]]

#3 Linux top crash for the 2-14 Nightly.

Comment 2

[Andrew McCreight [:mccr8]]

(In reply to Andrew McCreight [:mccr8] from comment #1)
> #3 Linux top crash for the 2-14 Nightly.

I accidentally included a 2-15 build in there, so this may not be accurate.

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Some investigation so far doesn't really show anything. The machines with fglrx I have show the detection working properly, mAllowSysVI being set properly when the policy is constructed, etc

Comment 4

[Gian-Carlo Pascutto [:gcp]]

Sandbox: seccomp sandbox violation: pid 3444, tid 3444, syscall 64, args 17908779 1 1974 140733924502176 0 0.  Killing process.
Sandbox: JS frame 0: pageLoaded https://get.webgl.org/ line 193
[Parent 3153, Gecko_IOThread] WARNING: pipe error (148): Connection reset by peer: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 353

Comment 5

[Gian-Carlo Pascutto [:gcp]]

GPU #1
Active	Yes
Description	ATI Technologies Inc. -- AMD Radeon(TM) R7 Graphics     
Vendor ID	ATI Technologies Inc.
Device ID	AMD Radeon(TM) R7 Graphics     
Driver Version	4.5.13399 Compatibility Profile Context 15.201.1151

Comment 6

[Gian-Carlo Pascutto [:gcp]]

https://searchfox.org/mozilla-central/source/security/sandbox/linux/launch/SandboxLaunch.cpp#244

We bail out of this function if there are no unprivileged user spaces (as is the case on this Debian). But the check and env variable setting for SysV IPC is later in this function, so we don't reach it.

Comment 7

[Gian-Carlo Pascutto [:gcp]]

Attachment: Bug 1438394 - Do SysV IPC check before bailing out early.

The SandboxLaunchPrepare currently bails out early if it detects a
lack of user namespaces. Hoist the check for drivers needing SysV
IPC up so it's done before that early exit, and the required env
variables get correctly set.

With this we no longer fail with a SIGSYS sandbox error, though
in a debug build we still crash because many assumptions in the
graphics stack get broken when that fails to initialize the driver
for WebGL.

Review commit: https://reviewboard.mozilla.org/r/225872/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/225872/

Comment 8

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

Comment on attachment 8956908 [details]
Bug 1438394 - Do SysV IPC check before bailing out early.

https://reviewboard.mozilla.org/r/225872/#review231766

::: security/sandbox/linux/launch/SandboxLaunch.cpp:244
(Diff revision 1)
>    // the child process whether this is the case.
>    aOptions->env_map["MOZ_SANDBOXED"] = "1";
>    PreloadSandboxLib(&aOptions->env_map);
>    AttachSandboxReporter(&aOptions->fds_to_remap);
>  
> +  bool needSysVIPC = false;

It looks like this variable isn't needed.

::: security/sandbox/linux/launch/SandboxLaunch.cpp:256
(Diff revision 1)
> +        // Tell the child process so it can adjust its seccomp-bpf
> +        // policy.
> +        aOptions->env_map["MOZ_SANDBOX_ALLOW_SYSV"] = "1";
> +        needSysVIPC = true;
> +      } else {
> +        flags |= CLONE_NEWIPC;

(I hadn't thought of moving the `CLONE_NEWIPC` up like this; it nicely avoids having the same test in two places.)

Comment 9

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8956908 [details]
Bug 1438394 - Do SysV IPC check before bailing out early.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/225872/diff/1-2/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9e92145bb572
Do SysV IPC check before bailing out early. r=jld

Comment 11

[Natalia Csoregi [:nataliaCs]]

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

Comment 12

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

For reference, to reproduce this on Ubuntu: `sudo sysctl -w kernel.unprivileged_userns_clone=0` or set `MOZ_ASSUME_USER_NS=0` in the environment.