media/gpu/linux: Verify a created GpuMemoryBufferHandle

CreateGpuMemoryBufferHandle() creates GpuMemoryBufferHandle from
media::VideoFrame(). The fds and metadata are come from untrusted processes,
e.g., ARC++ and renderer process. We must validate GpuMemoryBufferHandle.

Bug: None
Test: video_encode_accelerator_unittest on eve
Change-Id: I74faf5f9a38562d7120cc09d76dbaeb0d67515eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1839323
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Ricky Liang <jcliang@chromium.org>
Reviewed-by: Chih-Yu Huang <akahuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703614}

components/arc/video_accelerator/arc_video_accelerator_util.cc

@@ -4,82 +4,17 @@
 
 #include "components/arc/video_accelerator/arc_video_accelerator_util.h"
 
-#include <sys/types.h>
-#include <unistd.h>
-
 #include "base/files/file_util.h"
 #include "base/files/platform_file.h"
 #include "base/numerics/checked_math.h"
 #include "base/numerics/safe_conversions.h"
 #include "media/base/video_frame.h"
+#include "media/gpu/buffer_validation.h"
 #include "media/gpu/macros.h"
 #include "mojo/public/cpp/system/platform_handle.h"
 
 namespace arc {
 
-namespace {
-
-bool VerifyGpuMemoryBufferHandle(media::VideoPixelFormat pixel_format,
-                                 const gfx::Size& coded_size,
-                                 const gfx::GpuMemoryBufferHandle& gmb_handle) {
-  if (gmb_handle.type != gfx::NATIVE_PIXMAP) {
-    VLOGF(1) << "Unexpected GpuMemoryBufferType: " << gmb_handle.type;
-    return false;
-  }
-
-  const size_t num_planes = media::VideoFrame::NumPlanes(pixel_format);
-  if (num_planes != gmb_handle.native_pixmap_handle.planes.size() ||
-      num_planes == 0) {
-    VLOGF(1) << "Invalid number of dmabuf planes passed: "
-             << gmb_handle.native_pixmap_handle.planes.size()
-             << ", expected: " << num_planes;
-    return false;
-  }
-
-  // Strides monotonically decrease.
-  for (size_t i = 1; i < num_planes; i++) {
-    if (gmb_handle.native_pixmap_handle.planes[i - 1].stride <
-        gmb_handle.native_pixmap_handle.planes[i].stride) {
-      return false;
-    }
-  }
-
-  for (size_t i = 0; i < num_planes; i++) {
-    const auto& plane = gmb_handle.native_pixmap_handle.planes[i];
-    DVLOGF(4) << "Plane " << i << ", offset: " << plane.offset
-              << ", stride: " << plane.stride;
-
-    size_t file_size_in_bytes;
-    if (!plane.fd.is_valid() ||
-        !GetFileSize(plane.fd.get(), &file_size_in_bytes))
-      return false;
-
-    size_t plane_height =
-        media::VideoFrame::Rows(i, pixel_format, coded_size.height());
-    base::CheckedNumeric<size_t> min_plane_size =
-        base::CheckMul(plane.stride, plane_height);
-    if (!min_plane_size.IsValid() || min_plane_size.ValueOrDie() > plane.size) {
-      VLOGF(1) << "Invalid strides/sizes";
-      return false;
-    }
-
-    // Check |offset| + (the size of a plane) on each plane is not larger than
-    // |file_size_in_bytes|. This ensures we don't access out of a buffer
-    // referred by |fd|.
-    base::CheckedNumeric<size_t> min_buffer_size =
-        base::CheckAdd(plane.offset, plane.size);
-    if (!min_buffer_size.IsValid() ||
-        min_buffer_size.ValueOrDie() > file_size_in_bytes) {
-      VLOGF(1) << "Invalid strides/offsets";
-      return false;
-    }
-  }
-
-  return true;
-}
-
-}  // namespace
-
 base::ScopedFD UnwrapFdFromMojoHandle(mojo::ScopedHandle handle) {
   if (!handle.is_valid()) {
     VLOGF(1) << "Handle is invalid";
@@ -97,31 +32,6 @@ base::ScopedFD UnwrapFdFromMojoHandle(mojo::ScopedHandle handle) {
   return base::ScopedFD(platform_file);
 }
 
-bool GetFileSize(const int fd, size_t* size) {
-  if (fd < 0) {
-    VLOGF(1) << "Invalid file descriptor";
-    return false;
-  }
-
-  off_t fd_size = lseek(fd, 0, SEEK_END);
-  lseek(fd, 0, SEEK_SET);
-  if (fd_size < 0u) {
-    VPLOGF(1) << "Fail to find the size of fd";
-    return false;
-  }
-
-  if (!base::IsValueInRangeForNumericType<size_t>(fd_size)) {
-    VLOGF(1) << "fd_size is out of range of size_t"
-             << ", size=" << size
-             << ", size_t max=" << std::numeric_limits<size_t>::max()
-             << ", size_t min=" << std::numeric_limits<size_t>::min();
-    return false;
-  }
-
-  *size = static_cast<size_t>(fd_size);
-  return true;
-}
-
 std::vector<base::ScopedFD> DuplicateFD(base::ScopedFD fd, size_t num_fds) {
   if (!fd.is_valid()) {
     VLOGF(1) << "Input fd is not valid";
@@ -204,8 +114,9 @@ base::Optional<gfx::GpuMemoryBufferHandle> CreateGpuMemoryBufferHandle(
         stride, offset, size, std::move(scoped_fds[i]));
   }
 
-  if (!VerifyGpuMemoryBufferHandle(pixel_format, coded_size, gmb_handle))
+  if (!media::VerifyGpuMemoryBufferHandle(pixel_format, coded_size, gmb_handle))
     return base::nullopt;
+
   return gmb_handle;
 }
 

components/arc/video_accelerator/arc_video_accelerator_util.h

@@ -23,9 +23,6 @@ namespace arc {
 // Returns invalid ScopedFD on failure.
 base::ScopedFD UnwrapFdFromMojoHandle(mojo::ScopedHandle handle);
 
-// Return the file size of |fd| in bytes.
-bool GetFileSize(const int fd, size_t* size);
-
 // Return a list of duplicated |fd|. The size of list is |num_fds|. Return an
 // empty list if duplicatation fails.
 std::vector<base::ScopedFD> DuplicateFD(base::ScopedFD fd, size_t num_fds);

components/arc/video_accelerator/decoder_buffer.cc

@@ -11,6 +11,7 @@
 #include "base/unguessable_token.h"
 #include "components/arc/video_accelerator/arc_video_accelerator_util.h"
 #include "media/base/decoder_buffer.h"
+#include "media/gpu/buffer_validation.h"
 
 namespace arc {
 
@@ -52,7 +53,8 @@ scoped_refptr<media::DecoderBuffer> DecoderBuffer::ToMediaDecoderBuffer() && {
   }
 
   size_t file_size = 0;
-  if (!GetFileSize(handle_fd.get(), &file_size) || file_size < required_size) {
+  if (!media::GetFileSize(handle_fd.get(), &file_size) ||
+      file_size < required_size) {
     VLOG(1) << "File size(" << file_size << ") is smaller than required size("
             << required_size << ").";
     return nullptr;

components/arc/video_accelerator/gpu_arc_video_decode_accelerator.cc

@@ -13,6 +13,7 @@
 #include "components/arc/video_accelerator/protected_buffer_manager.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_types.h"
+#include "media/gpu/buffer_validation.h"
 #include "media/gpu/gpu_video_decode_accelerator_factory.h"
 #include "media/gpu/macros.h"
 #include "mojo/public/cpp/system/platform_handle.h"
@@ -370,7 +371,7 @@ void GpuArcVideoDecodeAccelerator::Decode(
     }
   } else {
     size_t handle_size;
-    if (!GetFileSize(handle_fd.get(), &handle_size)) {
+    if (!media::GetFileSize(handle_fd.get(), &handle_size)) {
       client_->NotifyError(
           mojom::VideoDecodeAccelerator::Result::INVALID_ARGUMENT);
       return;

components/arc/video_accelerator/gpu_arc_video_encode_accelerator.cc

@@ -17,6 +17,7 @@
 #include "media/base/color_plane_layout.h"
 #include "media/base/format_utils.h"
 #include "media/base/video_types.h"
+#include "media/gpu/buffer_validation.h"
 #include "media/gpu/gpu_video_encode_accelerator_factory.h"
 #include "media/gpu/macros.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
@@ -330,7 +331,7 @@ void GpuArcVideoEncodeAccelerator::UseBitstreamBuffer(
   }
 
   size_t shmem_size;
-  if (!GetFileSize(fd.get(), &shmem_size)) {
+  if (!media::GetFileSize(fd.get(), &shmem_size)) {
     client_->NotifyError(Error::kInvalidArgumentError);
     return;
   }

media/gpu/BUILD.gn

@@ -242,6 +242,8 @@ source_set("common") {
   defines = [ "MEDIA_GPU_IMPLEMENTATION" ]
   sources = [
     "accelerated_video_decoder.h",
+    "buffer_validation.cc",
+    "buffer_validation.h",
     "codec_picture.cc",
     "codec_picture.h",
     "gpu_video_decode_accelerator_helpers.cc",

media/gpu/buffer_validation.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "media/gpu/buffer_validation.h"
+
+#include "base/numerics/checked_math.h"
+#include "base/numerics/safe_conversions.h"
+#include "build/build_config.h"
+#include "media/base/video_frame.h"
+#include "media/gpu/macros.h"
+#include "ui/gfx/geometry/size.h"
+#include "ui/gfx/gpu_memory_buffer.h"
+
+#if defined(OS_LINUX)
+#include <sys/types.h>
+#include <unistd.h>
+#endif  // defined(OS_LINUX)
+
+namespace media {
+
+bool GetFileSize(const int fd, size_t* size) {
+#if defined(OS_LINUX)
+  if (fd < 0) {
+    VLOGF(1) << "Invalid file descriptor";
+    return false;
+  }
+
+  off_t fd_size = lseek(fd, 0, SEEK_END);
+  lseek(fd, 0, SEEK_SET);
+  if (fd_size < 0u) {
+    VPLOGF(1) << "Fail to find the size of fd";
+    return false;
+  }
+
+  if (!base::IsValueInRangeForNumericType<size_t>(fd_size)) {
+    VLOGF(1) << "fd_size is out of range of size_t"
+             << ", size=" << size
+             << ", size_t max=" << std::numeric_limits<size_t>::max()
+             << ", size_t min=" << std::numeric_limits<size_t>::min();
+    return false;
+  }
+
+  *size = static_cast<size_t>(fd_size);
+  return true;
+#else
+  NOTIMPLEMENTED();
+  return false;
+#endif  // defined(OS_LINUX)
+}
+
+bool VerifyGpuMemoryBufferHandle(media::VideoPixelFormat pixel_format,
+                                 const gfx::Size& coded_size,
+                                 const gfx::GpuMemoryBufferHandle& gmb_handle) {
+  if (gmb_handle.type != gfx::NATIVE_PIXMAP) {
+    VLOGF(1) << "Unexpected GpuMemoryBufferType: " << gmb_handle.type;
+    return false;
+  }
+#if defined(OS_LINUX)
+  const size_t num_planes = media::VideoFrame::NumPlanes(pixel_format);
+  if (num_planes != gmb_handle.native_pixmap_handle.planes.size() ||
+      num_planes == 0) {
+    VLOGF(1) << "Invalid number of dmabuf planes passed: "
+             << gmb_handle.native_pixmap_handle.planes.size()
+             << ", expected: " << num_planes;
+    return false;
+  }
+
+  // Strides monotonically decrease.
+  for (size_t i = 1; i < num_planes; i++) {
+    if (gmb_handle.native_pixmap_handle.planes[i - 1].stride <
+        gmb_handle.native_pixmap_handle.planes[i].stride) {
+      return false;
+    }
+  }
+
+  for (size_t i = 0; i < num_planes; i++) {
+    const auto& plane = gmb_handle.native_pixmap_handle.planes[i];
+    DVLOGF(4) << "Plane " << i << ", offset: " << plane.offset
+              << ", stride: " << plane.stride;
+
+    size_t file_size_in_bytes;
+    if (!plane.fd.is_valid() ||
+        !GetFileSize(plane.fd.get(), &file_size_in_bytes))
+      return false;
+
+    size_t plane_height =
+        media::VideoFrame::Rows(i, pixel_format, coded_size.height());
+    base::CheckedNumeric<size_t> min_plane_size =
+        base::CheckMul(plane.stride, plane_height);
+    if (!min_plane_size.IsValid() || min_plane_size.ValueOrDie() > plane.size) {
+      VLOGF(1) << "Invalid strides/sizes";
+      return false;
+    }
+
+    // Check |offset| + (the size of a plane) on each plane is not larger than
+    // |file_size_in_bytes|. This ensures we don't access out of a buffer
+    // referred by |fd|.
+    base::CheckedNumeric<size_t> min_buffer_size =
+        base::CheckAdd(plane.offset, plane.size);
+    if (!min_buffer_size.IsValid() ||
+        min_buffer_size.ValueOrDie() > file_size_in_bytes) {
+      VLOGF(1) << "Invalid strides/offsets";
+      return false;
+    }
+  }
+  return true;
+#else
+  NOTIMPLEMENTED();
+  return false;
+#endif  // defined(OS_LINUX)
+}
+
+}  // namespace media

media/gpu/buffer_validation.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef MEDIA_GPU_BUFFER_VALIDATION_H_
+#define MEDIA_GPU_BUFFER_VALIDATION_H_
+
+#include "media/base/video_types.h"
+#include "media/gpu/media_gpu_export.h"
+
+namespace gfx {
+class Size;
+struct GpuMemoryBufferHandle;
+}  // namespace gfx
+
+namespace media {
+// Gets the file size of |fd| and writes in |size|. Returns false on failure.
+MEDIA_GPU_EXPORT bool GetFileSize(const int fd, size_t* size);
+
+// Verifies if GpuMemoryBufferHandle is valid.
+MEDIA_GPU_EXPORT bool VerifyGpuMemoryBufferHandle(
+    media::VideoPixelFormat pixel_format,
+    const gfx::Size& coded_size,
+    const gfx::GpuMemoryBufferHandle& gmb_handle);
+
+}  // namespace media
+#endif  // MEDIA_GPU_BUFFER_VALIDATION_H_

media/gpu/linux/platform_video_frame_utils.cc

@@ -12,6 +12,7 @@
 #include "media/base/format_utils.h"
 #include "media/base/scopedfd_helper.h"
 #include "media/base/video_frame_layout.h"
+#include "media/gpu/buffer_validation.h"
 #include "media/gpu/macros.h"
 #include "ui/gfx/gpu_memory_buffer.h"
 #include "ui/gfx/linux/native_pixmap_dmabuf.h"
@@ -126,7 +127,6 @@ gfx::GpuMemoryBufferHandle CreateGpuMemoryBufferHandle(
     case VideoFrame::STORAGE_GPU_MEMORY_BUFFER:
       handle = video_frame->GetGpuMemoryBuffer()->CloneHandle();
       break;
-#if defined(OS_LINUX)
     case VideoFrame::STORAGE_DMABUFS: {
       handle.type = gfx::NATIVE_PIXMAP;
       std::vector<base::ScopedFD> duped_fds =
@@ -139,12 +139,16 @@ gfx::GpuMemoryBufferHandle CreateGpuMemoryBufferHandle(
         handle.native_pixmap_handle.planes.emplace_back(
             plane.stride, plane.offset, plane.size, std::move(duped_fds[i]));
       }
-#endif  // defined(OS_LINUX)
     } break;
     default:
       NOTREACHED() << "Unsupported storage type: "
                    << video_frame->storage_type();
   }
+  if (!handle.is_null() && handle.type == gfx::NATIVE_PIXMAP &&
+      !VerifyGpuMemoryBufferHandle(video_frame->format(),
+                                   video_frame->coded_size(), handle)) {
+    VLOGF(1) << "Created GpuMemoryBufferHandle is invalid";
+  }
   return handle;
 }