From 8c46973c3a1b6acd295f191436857a667ff2819b Mon Sep 17 00:00:00 2001
From: Sergio Solano <[email protected]>
Date: Wed, 15 Apr 2026 14:29:34 -0700
Subject: [PATCH] [ui/gfx] Fix NativePixmapPlane sign-extension and clamping issues

This is a quick fix for a potential security issue where the
NativePixmapPlane constructor and Mojo traits used signed 32-bit
integers. This led to sign-extension or clamping of offsets >= 2GB,
potentially causing memory corruption (b:497542537).

- Updates the constructor to use uint32_t and uint64_t to match
  internal member types.
- Updates relevant call sites in media/gpu and chromeos/ash to use the
  new constructor signature.
- Modernizes Mojo traits to remove redundant int casts and clamping
  (base::saturated_cast), allowing full 64-bit fidelity.
- Strengthens unit tests with a 2GB (0x80000000) offset check.
- Hardens test utility arithmetic using base::CheckMul.

For further reference: (Internal only)
go/code-terracotta-review-explainer

Project: Project-Fortify
Bug: b:497542537
Test: gfx_unittests --gtest_filter="StructTraitsTest.NativePixmapHandle"
Change-Id: I89545fa76f0638fc919194272e293131c6f4a200
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7718888
Reviewed-by: Tom Sepez <[email protected]>
Reviewed-by: Michael Spang <[email protected]>
Reviewed-by: Vasiliy Telezhnikov <[email protected]>
Commit-Queue: Sergio Solano <[email protected]>
Reviewed-by: Stephen Nusko <[email protected]>
Reviewed-by: Andres Calderon Jaramillo <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1615423}
---

diff --git a/chromeos/ash/experiences/arc/video_accelerator/protected_buffer_manager.cc b/chromeos/ash/experiences/arc/video_accelerator/protected_buffer_manager.cc
index 5f9f451..0161ead2 100644
--- a/chromeos/ash/experiences/arc/video_accelerator/protected_buffer_manager.cc
+++ b/chromeos/ash/experiences/arc/video_accelerator/protected_buffer_manager.cc
@@ -505,7 +505,7 @@
   // also on failure.
   gfx::NativePixmapHandle pixmap_handle;
   pixmap_handle.planes.emplace_back(
-      gfx::NativePixmapPlane(0, 0, 0u, std::move(dummy_fd)));
+      gfx::NativePixmapPlane(0u, 0u, 0u, std::move(dummy_fd)));
   ui::OzonePlatform* platform = ui::OzonePlatform::GetInstance();
   ui::SurfaceFactoryOzone* factory = platform->GetSurfaceFactoryOzone();
   scoped_refptr<gfx::NativePixmap> pixmap =
diff --git a/gpu/command_buffer/client/test_shared_image_interface.cc b/gpu/command_buffer/client/test_shared_image_interface.cc
index 224b06d..91f8bd56 100644
--- a/gpu/command_buffer/client/test_shared_image_interface.cc
+++ b/gpu/command_buffer/client/test_shared_image_interface.cc
@@ -16,6 +16,8 @@
 
 #include "base/check.h"
 #include "base/notreached.h"
+#include "base/numerics/checked_math.h"
+#include "base/numerics/safe_conversions.h"
 #include "build/build_config.h"
 #include "components/viz/common/resources/shared_image_format_utils.h"
 #include "gpu/command_buffer/client/client_shared_image.h"
@@ -440,7 +442,8 @@
         viz::SharedMemoryRowSizeForSharedImageFormat(format, i, size.width())
             .value();
     native_pixmap_handle.planes.emplace_back(
-        stride, 0, height_in_pixels * stride,
+        base::checked_cast<uint32_t>(stride), 0,
+        base::CheckMul(height_in_pixels, stride).ValueOrDie<uint64_t>(),
         base::ScopedFD(open("/dev/zero", O_RDWR)));
   }
 
diff --git a/media/gpu/chromeos/mock_native_pixmap_dmabuf.cc b/media/gpu/chromeos/mock_native_pixmap_dmabuf.cc
index 12fe2ab8..2fc8bed 100644
--- a/media/gpu/chromeos/mock_native_pixmap_dmabuf.cc
+++ b/media/gpu/chromeos/mock_native_pixmap_dmabuf.cc
@@ -59,8 +59,8 @@
       LOG(ERROR) << "Failed to open a file";
       return nullptr;
     }
-    handle.planes.emplace_back(base::checked_cast<int>(plane.stride),
-                               base::checked_cast<int>(plane.offset),
+    handle.planes.emplace_back(base::checked_cast<uint32_t>(plane.stride),
+                               base::strict_cast<uint64_t>(plane.offset),
                                base::strict_cast<uint64_t>(plane.size),
                                base::ScopedFD(file.TakePlatformFile()));
   }
diff --git a/media/gpu/chromeos/native_pixmap_frame_resource.cc b/media/gpu/chromeos/native_pixmap_frame_resource.cc
index b37d341..3554099 100644
--- a/media/gpu/chromeos/native_pixmap_frame_resource.cc
+++ b/media/gpu/chromeos/native_pixmap_frame_resource.cc
@@ -125,8 +125,8 @@
   handle.planes.reserve(num_planes);
   for (size_t i = 0; i < num_planes; ++i) {
     const auto& plane = layout.planes()[i];
-    handle.planes.emplace_back(base::checked_cast<int>(plane.stride),
-                               base::checked_cast<int>(plane.offset),
+    handle.planes.emplace_back(base::checked_cast<uint32_t>(plane.stride),
+                               base::strict_cast<uint64_t>(plane.offset),
                                base::strict_cast<uint64_t>(plane.size),
                                std::move(dmabuf_fds[i]));
   }
diff --git a/media/gpu/chromeos/platform_video_frame_utils.cc b/media/gpu/chromeos/platform_video_frame_utils.cc
index d337cd20..bf0e518 100644
--- a/media/gpu/chromeos/platform_video_frame_utils.cc
+++ b/media/gpu/chromeos/platform_video_frame_utils.cc
@@ -522,8 +522,8 @@
       for (size_t i = 0; i < num_planes; ++i) {
         const auto& plane = video_frame->layout().planes()[i];
         native_pixmap_handle.planes.emplace_back(
-            base::checked_cast<int>(plane.stride),
-            base::checked_cast<int>(plane.offset),
+            base::checked_cast<uint32_t>(plane.stride),
+            base::strict_cast<uint64_t>(plane.offset),
             base::strict_cast<uint64_t>(plane.size), std::move(duped_fds[i]));
       }
       handle = gfx::GpuMemoryBufferHandle(std::move(native_pixmap_handle));
diff --git a/media/gpu/test/test_gbm_buffer_manager.cc b/media/gpu/test/test_gbm_buffer_manager.cc
index ee71fed..5388fe94 100644
--- a/media/gpu/test/test_gbm_buffer_manager.cc
+++ b/media/gpu/test/test_gbm_buffer_manager.cc
@@ -104,8 +104,9 @@
   for (size_t i = 0;
        i < static_cast<size_t>(gbm_bo_get_plane_count(buffer_object)); ++i) {
     native_pixmap_handle.planes.push_back(gfx::NativePixmapPlane(
-        base::checked_cast<int>(gbm_bo_get_stride_for_plane(buffer_object, i)),
-        base::checked_cast<int>(gbm_bo_get_offset(buffer_object, i)),
+        base::checked_cast<uint32_t>(
+            gbm_bo_get_stride_for_plane(buffer_object, i)),
+        base::checked_cast<uint64_t>(gbm_bo_get_offset(buffer_object, i)),
         base::strict_cast<uint64_t>(gbm_bo_get_plane_size(buffer_object, i)),
         base::ScopedFD(gbm_bo_get_plane_fd(buffer_object, i))));
   }
diff --git a/ui/gfx/mojom/mojom_traits_unittest.cc b/ui/gfx/mojom/mojom_traits_unittest.cc
index 1aa11985..cc1c61d 100644
--- a/ui/gfx/mojom/mojom_traits_unittest.cc
+++ b/ui/gfx/mojom/mojom_traits_unittest.cc
@@ -20,9 +20,11 @@
 #include "ui/gfx/mojom/buffer_types_mojom_traits.h"
 #include "ui/gfx/mojom/hdr_metadata.mojom.h"
 #include "ui/gfx/mojom/hdr_metadata_mojom_traits.h"
+#include "ui/gfx/mojom/native_handle_types_mojom_traits.h"
 #include "ui/gfx/mojom/presentation_feedback.mojom.h"
 #include "ui/gfx/mojom/presentation_feedback_mojom_traits.h"
 #include "ui/gfx/mojom/traits_test_service.mojom.h"
+#include "ui/gfx/native_pixmap_handle.h"
 #include "ui/gfx/native_ui_types.h"
 #include "ui/gfx/selection_bound.h"
 
@@ -269,6 +271,28 @@
   EXPECT_EQ(input, output);
 }
 
+#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
+TEST_F(StructTraitsTest, NativePixmapHandle) {
+  // Test with a large offset that would trigger sign-extension if treated as
+  // int.
+  gfx::NativePixmapHandle native_pixmap_handle;
+  const uint32_t kStride = 1024;
+  const uint64_t kOffset = 0x80000000;
+  const uint64_t kSize = 4096;
+  native_pixmap_handle.planes.emplace_back(kStride, kOffset, kSize,
+                                           CreateValidLookingBufferHandle());
+
+  gfx::NativePixmapHandle output;
+  ASSERT_TRUE(
+      mojo::test::SerializeAndDeserialize<gfx::mojom::NativePixmapHandle>(
+          native_pixmap_handle, output));
+  ASSERT_FALSE(output.planes.empty());
+  EXPECT_EQ(kStride, output.planes[0].stride);
+  EXPECT_EQ(kOffset, output.planes[0].offset);
+  EXPECT_EQ(kSize, output.planes[0].size);
+}
+#endif
+
 TEST_F(StructTraitsTest, GpuMemoryBufferHandle) {
   const uint32_t kOffset = 126;
   const uint32_t kStride = 256;
diff --git a/ui/gfx/mojom/native_handle_types_mojom_traits.h b/ui/gfx/mojom/native_handle_types_mojom_traits.h
index 5bc99124..6101c5b 100644
--- a/ui/gfx/mojom/native_handle_types_mojom_traits.h
+++ b/ui/gfx/mojom/native_handle_types_mojom_traits.h
@@ -57,8 +57,8 @@
   static uint32_t stride(const gfx::NativePixmapPlane& plane) {
     return plane.stride;
   }
-  static int32_t offset(const gfx::NativePixmapPlane& plane) {
-    return base::saturated_cast<int32_t>(plane.offset);
+  static uint64_t offset(const gfx::NativePixmapPlane& plane) {
+    return plane.offset;
   }
   static uint64_t size(const gfx::NativePixmapPlane& plane) {
     return plane.size;
diff --git a/ui/gfx/native_pixmap_handle.cc b/ui/gfx/native_pixmap_handle.cc
index d4f1df1..46f9a25 100644
--- a/ui/gfx/native_pixmap_handle.cc
+++ b/ui/gfx/native_pixmap_handle.cc
@@ -33,8 +33,8 @@
 
 NativePixmapPlane::NativePixmapPlane() : stride(0), offset(0), size(0) {}

diff --git a/media/gpu/test/test_gbm_buffer_manager.cc b/media/gpu/test/test_gbm_buffer_manager.cc
index ee71fed..5388fe94 100644
--- a/media/gpu/test/test_gbm_buffer_manager.cc
+++ b/media/gpu/test/test_gbm_buffer_manager.cc
@@ -104,8 +104,9 @@
   for (size_t i = 0;
        i < static_cast<size_t>(gbm_bo_get_plane_count(buffer_object)); ++i) {
     native_pixmap_handle.planes.push_back(gfx::NativePixmapPlane(
-        base::checked_cast<int>(gbm_bo_get_stride_for_plane(buffer_object, i)),
-        base::checked_cast<int>(gbm_bo_get_offset(buffer_object, i)),
+        base::checked_cast<uint32_t>(
+            gbm_bo_get_stride_for_plane(buffer_object, i)),
+        base::checked_cast<uint64_t>(gbm_bo_get_offset(buffer_object, i)),
         base::strict_cast<uint64_t>(gbm_bo_get_plane_size(buffer_object, i)),
         base::ScopedFD(gbm_bo_get_plane_fd(buffer_object, i))));
   }
diff --git a/ui/gfx/mojom/mojom_traits_unittest.cc b/ui/gfx/mojom/mojom_traits_unittest.cc
index 1aa11985..cc1c61d 100644
--- a/ui/gfx/mojom/mojom_traits_unittest.cc
+++ b/ui/gfx/mojom/mojom_traits_unittest.cc
@@ -20,9 +20,11 @@
 #include "ui/gfx/mojom/buffer_types_mojom_traits.h"
 #include "ui/gfx/mojom/hdr_metadata.mojom.h"
 #include "ui/gfx/mojom/hdr_metadata_mojom_traits.h"
+#include "ui/gfx/mojom/native_handle_types_mojom_traits.h"
 #include "ui/gfx/mojom/presentation_feedback.mojom.h"
 #include "ui/gfx/mojom/presentation_feedback_mojom_traits.h"
 #include "ui/gfx/mojom/traits_test_service.mojom.h"
+#include "ui/gfx/native_pixmap_handle.h"
 #include "ui/gfx/native_ui_types.h"
 #include "ui/gfx/selection_bound.h"
 
@@ -269,6 +271,28 @@
   EXPECT_EQ(input, output);
 }
 
+#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
+TEST_F(StructTraitsTest, NativePixmapHandle) {
+  // Test with a large offset that would trigger sign-extension if treated as
+  // int.
+  gfx::NativePixmapHandle native_pixmap_handle;
+  const uint32_t kStride = 1024;
+  const uint64_t kOffset = 0x80000000;
+  const uint64_t kSize = 4096;
+  native_pixmap_handle.planes.emplace_back(kStride, kOffset, kSize,
+                                           CreateValidLookingBufferHandle());
+
+  gfx::NativePixmapHandle output;
+  ASSERT_TRUE(
+      mojo::test::SerializeAndDeserialize<gfx::mojom::NativePixmapHandle>(
+          native_pixmap_handle, output));
+  ASSERT_FALSE(output.planes.empty());
+  EXPECT_EQ(kStride, output.planes[0].stride);
+  EXPECT_EQ(kOffset, output.planes[0].offset);
+  EXPECT_EQ(kSize, output.planes[0].size);
+}
+#endif
+
 TEST_F(StructTraitsTest, GpuMemoryBufferHandle) {
   const uint32_t kOffset = 126;
   const uint32_t kStride = 256;