Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cherry-pick 99c3f3bfd507 from chromium #34225

Merged
merged 2 commits into from May 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/chromium/.patches
Expand Up @@ -135,3 +135,4 @@ cherry-pick-5be8e065f43e.patch
cherry-pick-12ba78f3fa7a.patch
reland_fix_noopener_case_for_user_activation_consumption.patch
fsa_pass_file_ownership_to_worker_for_async_fsarfd_file_operations.patch
cherry-pick-99c3f3bfd507.patch
64 changes: 64 additions & 0 deletions patches/chromium/cherry-pick-99c3f3bfd507.patch
@@ -0,0 +1,64 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Eugene Zemtsov <eugene@chromium.org>
Date: Fri, 8 Apr 2022 23:28:35 +0000
Subject: Only destroy successfully created compression session in VT encoder

This is a defensive change, since we don't have a repro on hand.
My guess is that VTCompressionSessionCreate() might fail to create a
compression session, but still write a value to compressionSessionOut.
It makes VTCompressionSessionInvalidate() access uninitialized memory.

That's why this CL makes sure that we only destroy a compression session
if VTCompressionSessionCreate() reports success.

Bug: 1312563
Change-Id: I468ce0e10bad251ca0b62b568607dbc5c32ba8bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3575680
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Eugene Zemtsov <eugene@chromium.org>
Cr-Commit-Position: refs/heads/main@{#990654}

diff --git a/media/gpu/mac/vt_video_encode_accelerator_mac.cc b/media/gpu/mac/vt_video_encode_accelerator_mac.cc
index 06333aaeca6b8bfd10d6809bcace4177ac7fd368..c265ff94617dfedca60ad9b3f1ce3de3364cfe56 100644
--- a/media/gpu/mac/vt_video_encode_accelerator_mac.cc
+++ b/media/gpu/mac/vt_video_encode_accelerator_mac.cc
@@ -129,13 +129,13 @@ VTVideoEncodeAccelerator::GetSupportedProfiles() {
SupportedProfiles profiles;
const bool rv = CreateCompressionSession(
gfx::Size(kDefaultResolutionWidth, kDefaultResolutionHeight));
- DestroyCompressionSession();
if (!rv) {
VLOG(1)
<< "Hardware encode acceleration is not available on this platform.";
return profiles;
}

+ DestroyCompressionSession();
SupportedProfile profile;
profile.max_framerate_numerator = kMaxFrameRateNumerator;
profile.max_framerate_denominator = kMaxFrameRateDenominator;
@@ -533,10 +533,8 @@ bool VTVideoEncodeAccelerator::ResetCompressionSession() {
DestroyCompressionSession();

bool session_rv = CreateCompressionSession(input_visible_size_);
- if (!session_rv) {
- DestroyCompressionSession();
+ if (!session_rv)
return false;
- }

const bool configure_rv = ConfigureCompressionSession();
if (configure_rv)
@@ -572,6 +570,12 @@ bool VTVideoEncodeAccelerator::CreateCompressionSession(
&VTVideoEncodeAccelerator::CompressionCallback,
reinterpret_cast<void*>(this), compression_session_.InitializeInto());
if (status != noErr) {
+ // IMPORTANT: ScopedCFTypeRef::release() doesn't call CFRelease().
+ // In case of an error VTCompressionSessionCreate() is not supposed to
+ // write a non-null value into compression_session_, but just in case,
+ // we'll clear it without calling CFRelease() because it can be unsafe
+ // to call on a not fully created session.
+ (void)compression_session_.release();
DLOG(ERROR) << " VTCompressionSessionCreate failed: " << status;
return false;
}