Bug 1546331 Comment 22 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

This is a simple thread interliving problem.  UAF can only happen on the nested worker case.

For top-level worker, instead of creating a RegisterDebuggerMainThreadRunnable, calling RegisterWorkerDebuggerMainThread directly, and debugger registration is completed before exiting WorkerPrivatre::Constructor and WorkerThreadPrimaryRunnable::Run. Therefore we never get the UAF on top-level worker case.

However, for nested workers, since the parent thread is not the main thread, WorkerPrivate::EnableDebugger() finally creating RegisterDebuggerMainThreadRunnable then dispatching it to main thread to register debugger. If there is no listeners on WorkerDebuggerManager, the parent thread is not blocked, and WorkerPrivate::Constructor continues the construction. 

https://searchfox.org/mozilla-esr68/source/dom/workers/WorkerDebuggerManager.cpp#247

That means WorkerThreadPrimaryRunnable::Run could be executed before RegisterDebuggerMainThreadRunnable::Run. For the situation, reload() could make WorkerThreadPrimaryRunnable::Run() exit the DoRunLoop immediately and call WorkerPrivate::ScheduleDeleteion. So if RegisterDebuggerMainThreadRunnable::Run is executed after that, we hit the use after free case.

To not change the WorkerPrivate lifecycle, followings could be the considered solutions.

1. Always call aWorkerPrivate->WaitForIsDebuggerRegistered(true) in the case, not only for the case if we have listeners on WorkerDebuggerManager. 
This simplest solution makes the WorkerPrivate construction behavior is the same with the top-level workers, but it introduces the overhead for the debugger is not using.
https://searchfox.org/mozilla-esr68/source/dom/workers/WorkerDebuggerManager.cpp#234

2. Cancel RegisterDebuggerMainThreadRunnable() if it is not executed when calling WorkerPrivate::ScheduleDeleteion
This solution is much more complicated for implementation. We need to remember the RegisterDebuggerMainThreadRunnable() in some where, probably in WorkerPrivate, and check is it executed when calling WorkerPrivate::ScheduleDeleteion() and cancel it at the mean time do not execute WorkerPrivate::DisableDebugger. Of course, I think Andrew's suggestion in comment 21 is also doable. I think it is also another way to cancel RegisterDebuggerMainThreadRunnable() by checking the WorkerPrivate status by itself.
This is a simple thread interleaving problem.  UAF can only happen on the nested worker case.

For top-level worker, instead of creating a RegisterDebuggerMainThreadRunnable, calling RegisterWorkerDebuggerMainThread directly, and debugger registration is completed before exiting WorkerPrivatre::Constructor and WorkerThreadPrimaryRunnable::Run. Therefore we never get the UAF on top-level worker case.

However, for nested workers, since the parent thread is not the main thread, WorkerPrivate::EnableDebugger() finally creating RegisterDebuggerMainThreadRunnable then dispatching it to main thread to register debugger. If there is no listeners on WorkerDebuggerManager, the parent thread is not blocked, and WorkerPrivate::Constructor continues the construction. 

https://searchfox.org/mozilla-esr68/source/dom/workers/WorkerDebuggerManager.cpp#247

That means WorkerThreadPrimaryRunnable::Run could be executed before RegisterDebuggerMainThreadRunnable::Run. For the situation, reload() could make WorkerThreadPrimaryRunnable::Run() exit the DoRunLoop immediately and call WorkerPrivate::ScheduleDeleteion. So if RegisterDebuggerMainThreadRunnable::Run is executed after that, we hit the use after free case.

To not change the WorkerPrivate lifecycle, followings could be the considered solutions.

1. Always call aWorkerPrivate->WaitForIsDebuggerRegistered(true) in the case, not only for the case if we have listeners on WorkerDebuggerManager. 
This simplest solution makes the WorkerPrivate construction behavior is the same with the top-level workers, but it introduces the overhead for the debugger is not using.
https://searchfox.org/mozilla-esr68/source/dom/workers/WorkerDebuggerManager.cpp#234

2. Cancel RegisterDebuggerMainThreadRunnable() if it is not executed when calling WorkerPrivate::ScheduleDeleteion
This solution is much more complicated for implementation. We need to remember the RegisterDebuggerMainThreadRunnable() in some where, probably in WorkerPrivate, and check is it executed when calling WorkerPrivate::ScheduleDeleteion() and cancel it at the mean time do not execute WorkerPrivate::DisableDebugger. Of course, I think Andrew's suggestion in comment 21 is also doable. I think it is also another way to cancel RegisterDebuggerMainThreadRunnable() by checking the WorkerPrivate status by itself.

Back to Bug 1546331 Comment 22