1285525 - Crash in mozilla::AudioStream::Init

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[Andreas Tolfsen ❲:ato❳]

Nightly crashes on attempting to initialise mozilla::AudioStream, which calls alsa_stream_init in libasound.so.2.0.0:

    https://crash-stats.mozilla.com/report/index/bd62f279-187a-42f0-b656-8fa032160708

This seems likely to be a fallout from bug 742434.

Comment 1

[Andreas Tolfsen ❲:ato❳]

This can be reproduced by being pinged on IRCCloud when the tab isn’t in the foreground.

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Attachment: 1285525 - Add sys_semget to seccomp-bpf whitelist.

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

Comment 4

[Julian Hector [:tedd] [:jhector]]

Comment on attachment 8769272 [details]
1285525 - Add sys_semget to seccomp-bpf whitelist.

https://reviewboard.mozilla.org/r/63236/#review60198

::: security/sandbox/linux/SandboxFilter.cpp:637
(Diff revision 1)
>          .Else(InvalidSyscall());
>      }
> +
> +    case __NR_semget:
> +      return Allow();
>  #endif

Is there a reason for this to be added within the __NR_rt_tsigqueueinfo ifdef?

I can't find sys_semget in syscall_32.tbl (x86 system call table), it probably needs its own ifdef if it is not defined on x86.

Comment 5

[Julian Hector [:tedd] [:jhector]]

:gcp what do you think of whitelisting the other system calls related to system v semaphore (all defined on x86_64, arm, arm64):

sys_semctl
sys_semget
sys_semop
sys_semtimedop

if sys_semget is used, the others are probably used as well. But we could also wait for crash reports to come in so we only whitelist the ones actually used.

Comment 6

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8769272 [details]
1285525 - Add sys_semget to seccomp-bpf whitelist.

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

Comment 7

[Gian-Carlo Pascutto [:gcp]]

(In reply to Julian Hector [:tedd] [:jhector] from comment #5)
> But we could also wait for crash reports to come in so we only whitelist the ones
> actually used.

I'd rather whitelist what we need. We can better keep track of the underlying callers that way.

Comment 8

[Julian Hector [:tedd] [:jhector]]

Comment on attachment 8769272 [details]
1285525 - Add sys_semget to seccomp-bpf whitelist.

https://reviewboard.mozilla.org/r/63236/#review60282

lgtm

Comment 9

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b46c6dcd1ea8bc355da21bf19bf212a9a7842e0
Bug 1285525 - Add sys_semget to seccomp-bpf whitelist. r=tedd

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4b46c6dcd1ea

Comment 11

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

https://reviewboard.mozilla.org/r/63236/#review60370

::: security/sandbox/linux/SandboxFilter.cpp:642
(Diff revision 2)
>          .Else(InvalidSyscall());
>      }
>  #endif
>  
> +#ifdef __NR_semget
> +    case __NR_semget:

This doesn't fix 32-bit x86; it should be checking for SEMGET in EvaluateIpcCall().

Comment 12

[Julian Hector [:tedd] [:jhector]]

Thanks Jed, I just checked dxr:
https://dxr.mozilla.org/mozilla-central/source/security/sandbox/linux/SandboxFilterUtil.cpp#100

It seems that __NR_semget is dispatched anyways, so just checking SEMGET in EvaluateIpcCall() should fix both architectures. Sorry :gcp I forgot that we dispatch those calls as well.

Comment 13

[Julian Hector [:tedd] [:jhector]]

I will fix this with the patch for Bug 1286033 (adding sys_semctl).