1620488 - Switching device in a row can lead to a UAF

Status: RESOLVED FIXED

(Core :: Audio/Video: cubeb, defect, P1)

Description

[C.M.Chang[:chunmin]]

If device-switching happens in a row, we may get the same UAF error in bug 1614971.

1. Start an output cubeb stream S
2. Switch the output device, a device-changed callback is fired on the thread C to dispatch the stream-reinit task R to the task queue Q that runs on thread T
3. Destroy the cubeb stream S on thread A, at the same time when stream-reinit task R is running on thread T
   1. The device-changed callbacks are not yet unregisted for stream S when S is being destroyed, on the thread A
   2. The device-changed callbacks are not yet registered for stream S, in the stream-reinit task R on thread T
4. The 3-i is done first. The device-changed callbacks are unregistered for stream S on thread A
5. The 3-ii is done next. The device-changed callbacks are registered for stream S, on thread T
6. The stream-reinit task R is done, while the stream destroy task D is not
7. Switch the output device again. Then the same error mentioned in bug 1614971 comment 3 and bug 1614971 comment 5 could show up again. Since the fix added in bug 1614971 here doesn't work when 3-ii is done after 3-i.
8. The device-changed callback is fired on the thread C
9. The stream is still being destroyed on thread A. It dispatches a stream-destroy task D to the task queue Q
10. Just after the stream-destroy task D is pushed to the task queue Q, the device-changed callback on the thread C pushes another stream-reinit task R' to the task queue Q.
11. The stream S is destroyed after the stream-destroy task D is done.
12. Then task queue Q will execute the stream-reinit task R'. Then a UAF error happens again since cubeb tries to reinitialize a destroyed stream S.

I didn't fix the UAF, caused by the data race, properly in bug 1614971 (yah blame me). The fix there fixes the UAF error happens when switching the device while the stream is being destroyed. If the device is switched in a row, in a few milliseconds, there is a slight chance to hit the same problem in bug 1614971 again. However, I guess it's nearly possible to do this manually. I need to add some delays in a few places in the code to make it happen.

This can be fixed in bug 1619005. Since all the tasks for stream S appended after the stream-destroyed task for stream S will be canceled.

Comment 1

[C.M.Chang[:chunmin]]

Plan to fix in this cycle

Comment 2

[C.M.Chang[:chunmin]]

The fix, cubeb-coreaudio-rs #55, is imported via D67536 in bug 1619005 comment 3, without mentioning anything about UAF .

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Do we need to uplift this to Beta for Fx75?

Comment 4

[C.M.Chang[:chunmin]]

The update changes the threading model, which means the risk is not low, so I'd like to test it Nightly and see what happens. Keeping the needinfo so I can keep my eyes on this.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

75 is in RC now. Let's let this ride 76.