ThreadSanitizer: data race [@ AsyncPanZoomController::SetParent] vs. [@ AsyncPanZoomController::GetParent]
Categories
(Core :: Panning and Zooming, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr78 | --- | unaffected |
firefox86 | --- | unaffected |
firefox87 | --- | unaffected |
firefox88 | --- | fixed |
People
(Reporter: Gankra, Assigned: hiro)
References
(Blocks 1 open bug, Regression)
Details
(Keywords: csectype-race, regression, sec-moderate, Whiteboard: [post-critsmash-triage])
Attachments
(2 files)
I can't really provide any interesting insights other than that clearly the Parent of an APZController is getting racily read on the main thread while it's being written on the compositor thread. This is a potential security issue since that's a race on the value of a RefPtr (and therefore a potential UAF).
Is there some sort of system in place to synchronize the compositor's and the main thread's accesses? Why is it not applicable here?
(This was discovered in the reports for Bug 1612600, after we fixed the original issue that had the same RefPtr signature, I haven't investigated for how long it's been happening.)
Permalinks to problematic lines:
Updated•3 years ago
|
Updated•3 years ago
|
Comment 1•3 years ago
|
||
Hiro, this is new since bug 1678505.
Calling AsyncPanZoomController::GetParent()
needs to be done while holding APZCTreeManager::mTreeLock
. Sorry for missing this during review.
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Comment 2•3 years ago
|
||
The cases where we need to walk up the APZC tree is when ScrollingDownWillMoveDynamicToolbar() returns true, here and here. So if ScrollingDownWillMoveDynamicToolbar() returns a pair of {boolean, const AsyncPanZoomController*} and the const AsyncPanZoomController*
is the handoffed APZC, we don't need to walk up the APZC tree there.
(In reply to Botond Ballo [:botond] from comment #1)
Calling
AsyncPanZoomController::GetParent()
needs to be done while holdingAPZCTreeManager::mTreeLock
. Sorry for missing this during review.
No problem, and thanks! TIL an important fundamental thing about APZ. :)
Assignee | ||
Comment 3•3 years ago
|
||
Updated•3 years ago
|
Updated•3 years ago
|
Comment 4•3 years ago
|
||
Make ScrollingDownWillMoveDynamicToolbar return a pair of boolean and cont AsyncPanZoomController*. r=botond
https://hg.mozilla.org/integration/autoland/rev/7cd1158b5e50b2a8b5a8d91e76a1545dd68b6210
https://hg.mozilla.org/mozilla-central/rev/7cd1158b5e50
Updated•3 years ago
|
Updated•2 years ago
|
Description
•