clock_gettime sandboxing rules incorrect for profiler
Categories
(Core :: Security: Process Sandboxing, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox98 | --- | fixed |
People
(Reporter: gerard-majax, Assigned: gerard-majax)
References
(Regressed 1 open bug)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
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
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
Assignee | ||
Comment 2•3 years ago
|
||
The side effect is that we end up calling
clock_gettime()
with clock values that needs to match6u
. While the rule4u | 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.
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.
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
.
Updated•3 years ago
|
Updated•3 years ago
|
Comment 6•3 years ago
|
||
bugherder |
Description
•