1673173 - PR_GetNumberOfProcessors probably still hits fallback cases

Status: NEW

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

Description

[Emilio Cobos Álvarez (:emilio)]

Per bug 1672482, ffmpeg decoder tries to use PR_GetNumberOfProcessors from the RDD process. That crashed before that bug in some libc versions, but it seems per the comments of https://phabricator.services.mozilla.com/D94358 it probably still needs more work:

…but it looks like the RDD process isn't allowed to open /sys/devices/system/cpu, so this may not do what you want. I think the crashing case is where sysconf races with sandbox startup: opens the directory before it starts, then calls readdir after it starts. (We've had other instances of that kind of race before.) If that's the case, PR_GetNumberOfProcessors still won't work when sandboxed.

Comment 1

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

The RDD policy actually does allow /sys/devices/system/cpu (and all descendents); when I wrote that Phabricator comment I must have been looking at an old version of the source (either locally or maybe on searchfox if it hadn't updated yet).

But see also bug 1672482 comment #10:

Looks like PR_GetNumberOfProcessors first tries to parse /sys/devices/system/cpu/present, and if that doesn't work falls back to sysconf( _SC_NPROCESSORS_CONF ). The latter then counts the number of cpu* subdirectories of /sys/devices/system/cpu which is what calls readdir. I guess we don't have to fall back in automation?

If something goes wrong with the CPU list parser, which looks a little delicate (is discontiguous CPU numbering possible?), then it falls back to sysconf which does the readdir; this can all happen strictly after sandbox startup and still be consistent with what we're observing.

Comment 2

[Gian-Carlo Pascutto [:gcp]]

To conclude from the above: given that we allow reading the dir and getdents now, the call will return the correct value because our fallback works. The question is why we hit the fallback; if we really want to know, we can assert in Nightly whenever we hit it, for example.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

doesn't seem to be a P1