Skip to content

Commit

Permalink
Implement NativePixmapHandle validation
Browse files Browse the repository at this point in the history
Now ClientNativePixmapDmaBuf and ScenicClientNativePixmapFactory
validate layout of the NativePixmapHandle to ensure that the buffer
fits the image.

Bug: 957314, 974375
Change-Id: I3a3a0c9a59ca2fd3658baed3d86c93a82c6bb54d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1831085
Reviewed-by: Michael Spang <spang@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701669}
  • Loading branch information
SergeyUlanov authored and Commit Bot committed Oct 1, 2019
1 parent b8ceae7 commit a751f0a
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 5 deletions.
3 changes: 2 additions & 1 deletion gpu/ipc/common/gpu_memory_buffer_impl_native_pixmap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ GpuMemoryBufferImplNativePixmap::CreateFromHandle(
std::unique_ptr<gfx::ClientNativePixmap> native_pixmap =
client_native_pixmap_factory->ImportFromHandle(
CloneHandleForIPC(handle.native_pixmap_handle), size, format, usage);
DCHECK(native_pixmap);
if (!native_pixmap)
return nullptr;

return base::WrapUnique(new GpuMemoryBufferImplNativePixmap(
handle.id, size, format, std::move(callback), std::move(native_pixmap),
Expand Down
3 changes: 2 additions & 1 deletion ui/gfx/client_native_pixmap_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ class GFX_EXPORT ClientNativePixmapFactory {
virtual ~ClientNativePixmapFactory() {}

// Import the native pixmap from |handle| to be used in non-GPU processes.
// This function takes ownership of any file descriptors in |handle|.
// Implementations must verify that the buffer in |handle| fits an image of
// the specified |size| and |format|. Otherwise nullptr is returned.
virtual std::unique_ptr<ClientNativePixmap> ImportFromHandle(
gfx::NativePixmapHandle handle,
const gfx::Size& size,
Expand Down
25 changes: 24 additions & 1 deletion ui/gfx/linux/client_native_pixmap_dmabuf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "base/strings/stringprintf.h"
#include "base/trace_event/trace_event.h"
#include "build/build_config.h"
#include "ui/gfx/buffer_format_util.h"
#include "ui/gfx/switches.h"

#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 11, 0)
Expand Down Expand Up @@ -172,11 +173,33 @@ bool ClientNativePixmapDmaBuf::IsConfigurationSupported(
// static
std::unique_ptr<gfx::ClientNativePixmap>
ClientNativePixmapDmaBuf::ImportFromDmabuf(gfx::NativePixmapHandle handle,
const gfx::Size& size) {
const gfx::Size& size,
gfx::BufferFormat format) {
std::array<PlaneInfo, kMaxPlanes> plane_info;

size_t expected_planes = gfx::NumberOfPlanesForLinearBufferFormat(format);
if (expected_planes == 0 || handle.planes.size() != expected_planes) {
return nullptr;
}

const size_t page_size = base::GetPageSize();
for (size_t i = 0; i < handle.planes.size(); ++i) {
// Verify that the plane buffer has appropriate size.
size_t min_stride = 0;
size_t subsample_factor = SubsamplingFactorForBufferFormat(format, i);
base::CheckedNumeric<size_t> plane_height =
(base::CheckedNumeric<size_t>(size.height()) + subsample_factor - 1) /
subsample_factor;
if (!gfx::RowSizeForBufferFormatChecked(size.width(), format, i,
&min_stride) ||
handle.planes[i].stride < min_stride) {
return nullptr;
}
base::CheckedNumeric<size_t> min_size =
base::CheckedNumeric<size_t>(handle.planes[i].stride) * plane_height;
if (!min_size.IsValid() || handle.planes[i].size < min_size.ValueOrDie())
return nullptr;

// mmap() fails if the offset argument is not page-aligned.
// Since handle.planes[i].offset is possibly not page-aligned, we
// have to map with an additional offset to be aligned to the page.
Expand Down
3 changes: 2 additions & 1 deletion ui/gfx/linux/client_native_pixmap_dmabuf.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ class ClientNativePixmapDmaBuf : public gfx::ClientNativePixmap {

static std::unique_ptr<gfx::ClientNativePixmap> ImportFromDmabuf(
gfx::NativePixmapHandle handle,
const gfx::Size& size);
const gfx::Size& size,
gfx::BufferFormat format);

~ClientNativePixmapDmaBuf() override;

Expand Down
2 changes: 1 addition & 1 deletion ui/gfx/linux/client_native_pixmap_factory_dmabuf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class ClientNativePixmapFactoryDmabuf : public ClientNativePixmapFactory {
case gfx::BufferUsage::CAMERA_AND_CPU_READ_WRITE:
case gfx::BufferUsage::SCANOUT_VEA_READ_CAMERA_AND_CPU_READ_WRITE:
return ClientNativePixmapDmaBuf::ImportFromDmabuf(std::move(handle),
size);
size, format);
case gfx::BufferUsage::GPU_READ:
case gfx::BufferUsage::SCANOUT:
case gfx::BufferUsage::SCANOUT_VDA_WRITE:
Expand Down
43 changes: 43 additions & 0 deletions ui/ozone/platform/scenic/client_native_pixmap_factory_scenic.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "base/fuchsia/fuchsia_logging.h"
#include "base/system/sys_info.h"
#include "ui/gfx/buffer_format_util.h"
#include "ui/gfx/client_native_pixmap.h"
#include "ui/gfx/client_native_pixmap_factory.h"
#include "ui/gfx/native_pixmap_handle.h"
Expand Down Expand Up @@ -107,6 +108,48 @@ class ScenicClientNativePixmapFactory : public gfx::ClientNativePixmapFactory {
const gfx::Size& size,
gfx::BufferFormat format,
gfx::BufferUsage usage) override {
// |planes| may be empty for non-mappable pixmaps. No need to validate the
// handle in that case.
if (handle.planes.empty())
return std::make_unique<ClientNativePixmapFuchsia>(std::move(handle));

size_t expected_planes = gfx::NumberOfPlanesForLinearBufferFormat(format);
if (handle.planes.size() != expected_planes)
return nullptr;

base::CheckedNumeric<size_t> vmo_size_checked =
base::CheckedNumeric<size_t>(handle.planes.back().offset) +
handle.planes.back().size;
if (!vmo_size_checked.IsValid()) {
return nullptr;
}
size_t vmo_size = vmo_size_checked.ValueOrDie();

// Validate plane layout and buffer size.
for (size_t i = 0; i < handle.planes.size(); ++i) {
size_t min_stride = 0;
size_t subsample_factor = SubsamplingFactorForBufferFormat(format, i);
base::CheckedNumeric<size_t> plane_height =
(base::CheckedNumeric<size_t>(size.height()) + subsample_factor - 1) /
subsample_factor;
if (!gfx::RowSizeForBufferFormatChecked(size.width(), format, i,
&min_stride) ||
handle.planes[i].stride < min_stride) {
return nullptr;
}

base::CheckedNumeric<size_t> min_size =
base::CheckedNumeric<size_t>(handle.planes[i].stride) * plane_height;
if (!min_size.IsValid() || handle.planes[i].size < min_size.ValueOrDie())
return nullptr;

base::CheckedNumeric<size_t> end_pos =
base::CheckedNumeric<size_t>(handle.planes[i].offset) +
handle.planes[i].size;
if (!end_pos.IsValid() || end_pos.ValueOrDie() > vmo_size)
return nullptr;
}

return std::make_unique<ClientNativePixmapFuchsia>(std::move(handle));
}

Expand Down

0 comments on commit a751f0a

Please sign in to comment.