media: fix heap-use-after-free in CobaltAudioRendererSink#9035
media: fix heap-use-after-free in CobaltAudioRendererSink#9035kjyoun merged 4 commits intoyoutube:mainfrom
Conversation
Use std::unique_ptr with a custom deleter for SbAudioSink to ensure the audio sink is properly destroyed when CobaltAudioRendererSink is destroyed. This prevents background ALSA threads from accessing the freed sink object. Fixes an ASAN error where UpdateSourceStatus was called on a freed CobaltAudioRendererSink instance. Bug: 483384414 Issue: 483384414
kjyoun
changed the title
🤖 Gemini Suggested Commit Message💡 Pro Tips for a Better Commit Message:
|
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves a heap-use-after-free vulnerability in CobaltAudioRendererSink by employing a std::unique_ptr with a custom deleter to manage the lifecycle of SbAudioSink. This change correctly applies the RAII principle to ensure the audio sink is always deallocated, even in error paths or during object destruction. The implementation is solid and directly addresses the issue. I have one suggestion to introduce a type alias for the unique_ptr, which would improve code readability and maintainability.
There was a problem hiding this comment.
Code Review
This pull request effectively addresses a heap-use-after-free bug by managing the SbAudioSink lifecycle with a std::unique_ptr and a custom deleter. This is a solid approach to ensure proper resource management. My review includes a few suggestions to align the implementation more closely with idiomatic C++ practices for handling C-style opaque pointers with smart pointers, which will improve code clarity and maintainability.
There was a problem hiding this comment.
Code Review
This pull request effectively resolves a heap-use-after-free issue by employing a std::unique_ptr with a custom deleter for managing the SbAudioSink lifecycle. This is a robust solution that aligns with modern C++ best practices for resource management. The implementation is clean and correct. I have one minor suggestion to improve consistency within the Flush() method.
Refactor CobaltAudioRendererSink to manage SbAudioSink lifecycle with std::unique_ptr and a custom deleter. This guarantees proper destruction of the audio sink when the renderer sink is destroyed or reset. This change prevents background ALSA threads from accessing a freed SbAudioSink object, addressing a heap-use-after-free ASAN error seen when UpdateSourceStatus was called on a freed instance. Issue: 483384414 (cherry picked from commit 7789ccf)
Refactor CobaltAudioRendererSink to manage SbAudioSink lifecycle with std::unique_ptr and a custom deleter. This guarantees proper destruction of the audio sink when the renderer sink is destroyed or reset. This change prevents background ALSA threads from accessing a freed SbAudioSink object, addressing a heap-use-after-free ASAN error seen when UpdateSourceStatus was called on a freed instance. Issue: 483384414 (cherry picked from commit 7789ccf)
…ndererSink (#9291) Refer to the original PR: #9035 Refactor CobaltAudioRendererSink to manage SbAudioSink lifecycle with std::unique_ptr and a custom deleter. This guarantees proper destruction of the audio sink when the renderer sink is destroyed or reset. This change prevents background ALSA threads from accessing a freed SbAudioSink object, addressing a heap-use-after-free ASAN error seen when UpdateSourceStatus was called on a freed instance. Issue: 483384414 Co-authored-by: Kyujung Youn <kjyoun@google.com>
…ndererSink (#9292) Refer to the original PR: #9035 Refactor CobaltAudioRendererSink to manage SbAudioSink lifecycle with std::unique_ptr and a custom deleter. This guarantees proper destruction of the audio sink when the renderer sink is destroyed or reset. This change prevents background ALSA threads from accessing a freed SbAudioSink object, addressing a heap-use-after-free ASAN error seen when UpdateSourceStatus was called on a freed instance. Issue: 483384414 Co-authored-by: Kyujung Youn <kjyoun@google.com>
2 participants
Refactor CobaltAudioRendererSink to manage SbAudioSink lifecycle
with std::unique_ptr and a custom deleter. This guarantees proper
destruction of the audio sink when the renderer sink is destroyed or
reset.
This change prevents background ALSA threads from accessing a freed
SbAudioSink object, addressing a heap-use-after-free ASAN error seen
when UpdateSourceStatus was called on a freed instance.
Issue: 483384414