diff options
author | Dale Curtis <dalecurtis@chromium.org> | 2023-02-20 13:36:56 +0000 |
---|---|---|
committer | Michael BrĂ¼ning <michael.bruning@qt.io> | 2023-02-27 18:32:47 +0000 |
commit | be6ac82e2bfe2ef9d00992bb02dda09be0ff25d4 (patch) | |
tree | fb09ac64f0d5295be67c98458ec85f3692320907 | |
parent | bdc50b6d92a4f79af98a78ba3c2dd3a63fefba40 (diff) | |
download | qtwebengine-chromium-be6ac82e2bfe2ef9d00992bb02dda09be0ff25d4.tar.gz |
[Backport] CVE-2023-0931: Use after free in Video (1/2)
Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/4227731:
[M102-LTS] Simplify WebMediaPlayerMSCompositor destruction.
The code was only sometimes calling StopUsingProvider() and posted
the submitter destruction unnecessarily.
Destruction now works the same as in VideoFrameCompositor, where the
class itself is responsible for calling StopUsingProvider() during
its own destruction.
(cherry picked from commit cbd238e85903b7d94910bd2c6362ff9abf9908cc)
Fixed: 1407701
Change-Id: Ia649cb5532519468eea34e12745ed9c990580d82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4195824
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Tony Herre <toprice@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1098505}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4227731
Owners-Override: Michael Ershov <miersh@google.com>
Commit-Queue: Roger Felipe Zanoni da Silva <rzanoni@google.com>
Reviewed-by: Michael Ershov <miersh@google.com>
Cr-Commit-Position: refs/branch-heads/5005@{#1435}
Cr-Branched-From: 5b4d9450fee01f821b6400e947b3839727643a71-refs/heads/main@{#992738}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/462798
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
3 files changed, 2 insertions, 30 deletions
diff --git a/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms.cc b/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms.cc index 48206460a62..6d035b2e55d 100644 --- a/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms.cc +++ b/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms.cc @@ -379,9 +379,6 @@ WebMediaPlayerMS::~WebMediaPlayerMS() { if (frame_deliverer_) io_task_runner_->DeleteSoon(FROM_HERE, frame_deliverer_.release()); - if (compositor_) - compositor_->StopUsingProvider(); - if (video_frame_provider_) video_frame_provider_->Stop(); diff --git a/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.cc b/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.cc index a296140c1d0..0763e167be4 100644 --- a/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.cc +++ b/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.cc @@ -207,12 +207,8 @@ WebMediaPlayerMSCompositor::~WebMediaPlayerMSCompositor() { // Ensured by destructor traits. DCHECK(video_frame_compositor_task_runner_->BelongsToCurrentThread()); - if (submitter_) { - video_frame_compositor_task_runner_->DeleteSoon(FROM_HERE, - std::move(submitter_)); - } else { - DCHECK(!video_frame_provider_client_) - << "Must call StopUsingProvider() before dtor!"; + if (video_frame_provider_client_) { + video_frame_provider_client_->StopUsingProvider(); } } @@ -486,15 +482,6 @@ void WebMediaPlayerMSCompositor::ReplaceCurrentFrameWithACopy() { WrapRefCounted(this)))); } -void WebMediaPlayerMSCompositor::StopUsingProvider() { - DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - PostCrossThreadTask( - *video_frame_compositor_task_runner_, FROM_HERE, - CrossThreadBindOnce( - &WebMediaPlayerMSCompositor::StopUsingProviderInternal, - WrapRefCounted(this))); -} - bool WebMediaPlayerMSCompositor::MapTimestampsToRenderTimeTicks( const std::vector<base::TimeDelta>& timestamps, std::vector<base::TimeTicks>* wall_clock_times) { @@ -719,13 +706,6 @@ void WebMediaPlayerMSCompositor::StopRenderingInternal() { video_frame_provider_client_->StopRendering(); } -void WebMediaPlayerMSCompositor::StopUsingProviderInternal() { - DCHECK(video_frame_compositor_task_runner_->BelongsToCurrentThread()); - if (video_frame_provider_client_) - video_frame_provider_client_->StopUsingProvider(); - video_frame_provider_client_ = nullptr; -} - void WebMediaPlayerMSCompositor::ReplaceCurrentFrameWithACopyInternal() { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); scoped_refptr<media::VideoFrame> current_frame_ref; diff --git a/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.h b/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.h index 10d01b8528e..ae0a1a39f9f 100644 --- a/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.h +++ b/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.h @@ -109,10 +109,6 @@ class MODULES_EXPORT WebMediaPlayerMSCompositor void StopRendering(); void ReplaceCurrentFrameWithACopy(); - // Tell |video_frame_provider_client_| to stop using this instance in - // preparation for dtor. - void StopUsingProvider(); - // Sets a hook to be notified when a new frame is presented, to fulfill a // prending video.requestAnimationFrame() request. // Can be called from any thread. @@ -195,7 +191,6 @@ class MODULES_EXPORT WebMediaPlayerMSCompositor void StartRenderingInternal(); void StopRenderingInternal(); - void StopUsingProviderInternal(); void ReplaceCurrentFrameWithACopyInternal(); void SetAlgorithmEnabledForTesting(bool algorithm_enabled); |