Bug 188281

Summary: JSLock::ownerThread seems racy
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: NEW    
Severity: Normal CC: ap, benjamin, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, rmorisset, ticaiolima, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   

Saam Barati
Reported 2018-08-02 15:35:26 PDT
Specifically, the race I'm worried about is: ``` m_ownerThread = ...; // set to non null value some time long ago. T1: VMTraps signal sender thread. T2: VM main execution thread. T1: tmp1 = m_ownerThread.m_ptr; T2: tmp2 = m_ownerThread.m_ptr; T2: m_ownerThread.m_ptr = nullptr; T2: tmp2->deref(); // ref count going from 1->0 T2: ~(tmp2); T1: tmp1->ref(); // UAF ``` Would be helpful if somebody else looked to see if there are errors in my logic here.
Attachments
Saam Barati
Comment 1 2018-08-02 15:43:20 PDT
(In reply to Saam Barati from comment #0) > Specifically, the race I'm worried about is: > > ``` > m_ownerThread = ...; // set to non null value some time long ago. > T1: VMTraps signal sender thread. > T2: VM main execution thread. > > T1: tmp1 = m_ownerThread.m_ptr; > T2: tmp2 = m_ownerThread.m_ptr; > T2: m_ownerThread.m_ptr = nullptr; > T2: tmp2->deref(); // ref count going from 1->0 > T2: ~(tmp2); > T1: tmp1->ref(); // UAF > ``` > > Would be helpful if somebody else looked to see if there are errors in my > logic here. actually, it'd look somewhat different, since we never null out that pointer, we just assign new pointers in, but race still holds I think: ``` m_ownerThread = ...; // set to non null a long time ago T1: VMTraps signal sender thread. T2: VM main execution thread. T1: tmp1 = m_ownerThread.m_ptr; T2: tmp2 = m_ownerThread.m_ptr; T2: m_ownerThread.m_ptr = some other ptr; T2: tmp2->deref(); // ref count going from 1->0 T2: ~(tmp2); T1: tmp1->ref(); // UAF ```
Radar WebKit Bug Importer
Comment 2 2019-01-20 14:50:33 PST
Note You need to log in before you can comment on or make changes to this bug.