Closed
Bug 1308423
Opened 9 years ago
Closed 9 years ago
[App Verifier] Critical section not initialized in cubeb
Categories
(Core :: Audio/Video: cubeb, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: cyu, Assigned: cyu)
References
(Blocks 1 open bug)
Details
(Whiteboard: app_verifier)
Attachments
(2 files, 1 obsolete file)
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.
Comment hidden (obsolete) |
Assignee | ||
Comment 2•9 years ago
|
||
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.
Comment hidden (mozreview-request) |
Comment 4•9 years ago
|
||
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.
Updated•9 years ago
|
Attachment #8798823 -
Flags: review?(kinetik)
Updated•9 years ago
|
Rank: 10
Priority: -- → P1
Assignee | ||
Updated•9 years ago
|
Attachment #8798823 -
Attachment is obsolete: true
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8799627 -
Flags: review?(kinetik)
Assignee | ||
Comment 6•9 years ago
|
||
Comment 7•9 years ago
|
||
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+
Comment 8•9 years ago
|
||
Cervantes merged fix was uplifted into Gecko in bug 1308418.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Updated•8 years ago
|
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•