From c0244cac701a5bfadcd275dc10ee3d12ef593908 Mon Sep 17 00:00:00 2001
From: Eugene Zemtsov <[email protected]>
Date: Mon, 02 Mar 2026 19:16:41 -0800
Subject: [PATCH] media: Zero-initialize frames in software decoders to prevent info leaks

Pass `zero_initialize_memory=true` when creating FrameBufferPools for
libvpx and dav1d decoders. Also update AllocateAlphaPlaneForFrameBuffer
to respect this flag. This prevents potential heap information
disclosure from uninitialized padding bytes.

We already do it for ffmpeg decoder.

Bug: 486506202
Change-Id: I5e88f827f7043cfebef140524092e5543d7d03a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7616614
Reviewed-by: Dale Curtis <[email protected]>
Commit-Queue: Eugene Zemtsov <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1592929}
---

diff --git a/media/base/frame_buffer_pool.cc b/media/base/frame_buffer_pool.cc
index e90f070..ceb0313c 100644
--- a/media/base/frame_buffer_pool.cc
+++ b/media/base/frame_buffer_pool.cc
@@ -63,6 +63,33 @@
   base::TimeTicks last_use_time;
 };
 
+namespace {
+
+BytesArray AllocateMemory(size_t min_size,
+                          bool zero_initialize,
+                          bool force_error) {
+  if (force_error) {
+    return {};
+  }
+
+  uint8_t* data = nullptr;
+  const bool result =
+      zero_initialize
+          ? base::UncheckedCalloc(1u, min_size, reinterpret_cast<void**>(&data))
+          : base::UncheckedMalloc(min_size, reinterpret_cast<void**>(&data));
+
+  // Unclear why, but the docs indicate both that `data` will be null on
+  // failure, and also that the return value must not be discarded.
+  if (!result || !data) {
+    return {};
+  }
+
+  // SAFETY: We have just allocated `min_size` of memory for `data`.
+  return UNSAFE_BUFFERS(BytesArray::FromOwningPointer(data, min_size));
+}
+
+}  // namespace
+
 FrameBufferPool::FrameBufferPool(bool zero_initialize_memory)
     : zero_initialize_memory_(zero_initialize_memory),
       tick_clock_(base::DefaultTickClock::GetInstance()) {}
@@ -98,35 +125,15 @@
   frame_buffer->held_by_library = true;
   if (frame_buffer->data.size() < min_size) {
     // Free the existing |data| first so that the memory can be reused,
-    // if possible. Note that the new array is purposely not initialized.
+    // if possible.
     frame_buffer->data = {};
+    frame_buffer->data = AllocateMemory(min_size, zero_initialize_memory_,
+                                        force_allocation_error_);
 
-    uint8_t* data = nullptr;
-    if (!force_allocation_error_) {
-      bool result = false;
-      if (zero_initialize_memory_) {
-        result = base::UncheckedCalloc(1u, min_size,
-                                       reinterpret_cast<void**>(&data));
-      } else {
-        result =
-            base::UncheckedMalloc(min_size, reinterpret_cast<void**>(&data));
-      }
-
-      // Unclear why, but the docs indicate both that `data` will be null on
-      // failure, and also that the return value must not be discarded.
-      if (!result) {
-        data = nullptr;
-      }
-    }
-
-    if (!data) {
+    if (frame_buffer->data.empty()) {
       frame_buffers_.erase(it);
       return {};
     }
-
-    // SAFETY: We have just allocated `min_size` of memory for `data`.
-    frame_buffer->data =
-        UNSAFE_BUFFERS(BytesArray::FromOwningPointer(data, min_size));
   }
 
   // Provide the client with a private identifier.
@@ -158,17 +165,10 @@
   DCHECK(IsUsedLocked(frame_buffer));
   if (frame_buffer->alpha_data.size() < min_size) {
     // Free the existing |alpha_data| first so that the memory can be reused,
-    // if possible. Note that the new array is purposely not initialized.
+    // if possible.
     frame_buffer->alpha_data = {};
-    uint8_t* data = nullptr;
-    if (force_allocation_error_ ||
-        !base::UncheckedMalloc(min_size, reinterpret_cast<void**>(&data)) ||
-        !data) {
-      return {};
-    }
-    // SAFETY: We have just allocated `min_size` of memory for `data`.
-    frame_buffer->alpha_data =
-        UNSAFE_BUFFERS(BytesArray::FromOwningPointer(data, min_size));
+    frame_buffer->alpha_data = AllocateMemory(min_size, zero_initialize_memory_,
+                                              force_allocation_error_);
   }
   return frame_buffer->alpha_data;
 }
diff --git a/media/base/frame_buffer_pool_unittest.cc b/media/base/frame_buffer_pool_unittest.cc
index a5b7bff2..893e941 100644
--- a/media/base/frame_buffer_pool_unittest.cc
+++ b/media/base/frame_buffer_pool_unittest.cc
@@ -131,6 +131,14 @@
     nonzero |= !!buf[i];
   }
   EXPECT_FALSE(nonzero);
+
+  auto alpha_buf = pool->AllocateAlphaPlaneForFrameBuffer(kBufferSize, priv1);
+  nonzero = false;
+  for (size_t i = 0; i < kBufferSize; i++) {
+    nonzero |= !!alpha_buf[i];
+  }
+  EXPECT_FALSE(nonzero);
+
   pool->Shutdown();
 }
 
diff --git a/media/filters/dav1d_video_decoder.cc b/media/filters/dav1d_video_decoder.cc
index baef967..3456cd5 100644
--- a/media/filters/dav1d_video_decoder.cc
+++ b/media/filters/dav1d_video_decoder.cc
@@ -285,7 +285,8 @@
   }
 
   if (!frame_pool_) {
-    frame_pool_ = base::MakeRefCounted<FrameBufferPool>();
+    frame_pool_ =
+        base::MakeRefCounted<FrameBufferPool>(/*zero_initialize_memory=*/true);
   }
 
   // Clear any previously initialized decoder.
diff --git a/media/filters/vpx_video_decoder.cc b/media/filters/vpx_video_decoder.cc
index 14b4044..d197a8feb 100644
--- a/media/filters/vpx_video_decoder.cc
+++ b/media/filters/vpx_video_decoder.cc
@@ -245,7 +245,8 @@
            VPX_CODEC_CAP_EXTERNAL_FRAME_BUFFER);
 
     DCHECK(!memory_pool_);
-    memory_pool_ = base::MakeRefCounted<FrameBufferPool>();
+    memory_pool_ =
+        base::MakeRefCounted<FrameBufferPool>(/*zero_initialize_memory=*/true);
 
     if (vpx_codec_set_frame_buffer_functions(
             vpx_codec_.get(), &GetVP9FrameBuffer, &ReleaseVP9FrameBuffer,

diff --git a/media/base/frame_buffer_pool_unittest.cc b/media/base/frame_buffer_pool_unittest.cc
index a5b7bff2..893e941 100644
--- a/media/base/frame_buffer_pool_unittest.cc
+++ b/media/base/frame_buffer_pool_unittest.cc
@@ -131,6 +131,14 @@
     nonzero |= !!buf[i];
   }
   EXPECT_FALSE(nonzero);
+
+  auto alpha_buf = pool->AllocateAlphaPlaneForFrameBuffer(kBufferSize, priv1);
+  nonzero = false;
+  for (size_t i = 0; i < kBufferSize; i++) {
+    nonzero |= !!alpha_buf[i];
+  }
+  EXPECT_FALSE(nonzero);
+
   pool->Shutdown();
 }