Closed Bug 1308423 Opened 5 years ago Closed 5 years ago
[App Verifier] Critical section not initialized in cubeb
In xul!`anonymous namespace'::wasapi_stream_init+231 (e:\hg\mozilla-central\media\libcubeb\src\cubeb_wasapi.cpp @ 1675), cubeb copies owned_critical_section(), which is a wrapper around the windows CRITICAL_SECTION, where on MSDN, "A critical section object cannot be moved or copied.": https://msdn.microsoft.com/zh-tw/library/windows/desktop/ms683472(v=vs.85).aspx The danger of copying a CRITICAL_SECTION is that the temporary object is destroyed with DELETE_CRITICAL_SECTION, with its internal implementation containing HANDLEs and pointers copied to stm->stream_reset_lock. This may result in some use-after-free of pointers or system calls using invalid HANDLES.
For more information, owned_critical_section is copied in https://dxr.mozilla.org/mozilla-central/rev/da986c9f1f723af1e0c44f4ccd4cddd5fb6084e8/media/libcubeb/src/cubeb_wasapi.cpp#1645 The posix version of owned_critical_section() has the same problem.
Thanks, this looks good. Would you mind submitting it as a PR against https://github.com/kinetiknz/cubeb so we can merge it there and then update in Gecko? It's probably a good idea to make the copy constructor private on owned_critical_section to prevent other users from making the same mistake, too.
Attachment #8798823 - Attachment is obsolete: true
Comment on attachment 8799627 [details] [review] Don't copy owned_critical_section I merged this in https://github.com/kinetiknz/cubeb/commit/22557d466eceb6ff6ba70ae30d2dcd87648cde0b Thanks for the PR!
Attachment #8799627 - Flags: review?(kinetik) → review+
Cervantes merged fix was uplifted into Gecko in bug 1308418.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.