1749606 - clock_gettime sandboxing rules incorrect for profiler

Status: RESOLVED FIXED

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

Description

[:gerard-majax]

In the current rule, we extract from clock value via clock & 7u and we compare to expect values 4u | 2u.

However, from https://github.com/torvalds/linux/blob/v5.16/include/linux/posix-timers.h#L29-L48 we can see that make_thread_cpuclock takes the value and | it with CPUCLOCK_PERTHREAD_MASK (4). Code uses then https://github.com/torvalds/linux/blob/v5.16/kernel/time/posix-cpu-timers.c#L1572 so we end up doing:

• make_thread_cpuclock(..., CPUCLOCK_SCHED | CPUCLOCK_PERTHREAD_MASK)

Those kernel defines are being redefined in https://searchfox.org/mozilla-central/rev/7271a078fa0c1b858a52614ea60ac82fdd8b3d23/toolkit/components/processtools/ProcInfo_linux.cpp#22-35 and we use them the same way:

• https://searchfox.org/mozilla-central/rev/d107bc8aeadcc816ba85cb21c1a6a1aac1d4ef9f/toolkit/components/processtools/ProcInfo_linux.cpp#237
• https://searchfox.org/mozilla-central/rev/d107bc8aeadcc816ba85cb21c1a6a1aac1d4ef9f/toolkit/components/processtools/ProcInfo_linux.cpp#304

The side effect is that we end up calling clock_gettime() with clock values that needs to match 6u. While the rule 4u | 2u should match that, in practice, it seems not to hold and we end up with child-side sandbox violation

Comment 1

[:gerard-majax]

Attachment: Bug 1749606 - Allow clock_gettime() for same-process r?jld!,gerald!

Comment 2

[:gerard-majax]

The side effect is that we end up calling clock_gettime() with clock values that needs to match 6u. While the rule 4u | 2u should match that, in practice, it seems not to hold and we end up with child-side sandbox violation

To make it more clear, while working on https://bugzilla.mozilla.org/show_bug.cgi?id=1731890 I hit some gtest failure https://treeherder.mozilla.org/jobs?repo=try&revision=b6705362c6cb1de16ce751d533b19d68eeb5af53&selectedTaskRun=S457CYuTSqa932qTP31JuQ.0

This was because some EmptyUtil started on the UtilityProcess was running, and being caught by the SamplerThread code from the profiler trying to profile unregistered threads. This ends up doing measurements in https://searchfox.org/mozilla-central/rev/d107bc8aeadcc816ba85cb21c1a6a1aac1d4ef9f/toolkit/components/processtools/ProcInfo_linux.cpp#237,304 that were intercepted by the sandbox.

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

I admit my knowledge of both Linux and the sandbox are limited, so it's very possible these changes were not quite correct, and that's why some new usage can break them now.
If your attached patch helps (and you have done good research), I'm happy with it! Please check with a(nother) sandbox expert.

Please let me know if there's something to be done in the Profiler code itself.

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

To confirm that it's expected: The Profiler uses this to know how much time the current process and its threads have spent on the CPU.

In case it's useful:
Note that we're also using clock_gettime() for registered threads, but the clockid_t parameter for them was found earlier from each thread itself through pthread_getcpuclockid(pthread_self()) instead of constructing it with MAKE_THREAD_CPUCLOCK.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6f3c07119e59
Allow clock_gettime() for same-process r=jld,gerald

Comment 6

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/6f3c07119e59