Add Metafile interface to get data as shared memory region

Transferring print job data from the browser process to the Print
Backend service is done using a shared memory region to improve
performance.  Introduce a method to get the print job data from a
metafile in this format for the browser (used by PrintJobWorker in
https://crrev.com/c/3193950).  The Print Backend service will read from
this shared region to then submit the job data to the system printing
APIs.

Included is a query to determine if cross-process Metafile users should
make a copy of data from a shared memory region before initializing the
metafile.  This is related to security concerns for operating directly
on data from shared memory (crbug.com/1135729).  Callers need not make
an extra copy for implementations which automatically make copies of
the provided data.

Bug: 809738
Change-Id: Iae169dab77ea32053d37327ae064989bf0ae0c25
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3183192
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Alan Screen <awscreen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#947585}

printing/emf_win.cc

@@ -158,6 +158,13 @@ bool Emf::GetData(void* buffer, uint32_t size) const {
   return size2 == size && size2 != 0;
 }
 
+bool Emf::ShouldCopySharedMemoryRegionData() const {
+  // `InitFromData()` operates directly upon memory provide to it, so any
+  // caller for cases where this data is shared cross-process should have the
+  // data copied before it is operated upon.
+  return true;
+}
+
 mojom::MetafileDataType Emf::GetDataType() const {
   return mojom::MetafileDataType::kEMF;
 }

printing/emf_win.h

@@ -70,6 +70,7 @@ class COMPONENT_EXPORT(PRINTING_METAFILE) Emf : public Metafile {
 
   uint32_t GetDataSize() const override;
   bool GetData(void* buffer, uint32_t size) const override;
+  bool ShouldCopySharedMemoryRegionData() const override;
   mojom::MetafileDataType GetDataType() const override;
 
   // Should be passed to Playback to keep the exact same size.

printing/metafile.cc

@@ -10,6 +10,7 @@
 
 #include "base/files/file.h"
 #include "base/logging.h"
+#include "base/memory/read_only_shared_memory_region.h"
 #include "base/numerics/safe_conversions.h"
 
 namespace printing {
@@ -30,6 +31,29 @@ bool Metafile::GetDataAsVector(std::vector<char>* buffer) const {
                  base::checked_cast<uint32_t>(buffer->size()));
 }
 
+base::MappedReadOnlyRegion Metafile::GetDataAsSharedMemoryRegion() const {
+  uint32_t data_size = GetDataSize();
+  if (data_size == 0) {
+    DLOG(ERROR) << "Metafile has no data to map to a region.";
+    return base::MappedReadOnlyRegion();
+  }
+
+  base::MappedReadOnlyRegion region_mapping =
+      base::ReadOnlySharedMemoryRegion::Create(data_size);
+  if (!region_mapping.IsValid()) {
+    DLOG(ERROR) << "Failure mapping metafile data into region for size "
+                << data_size;
+    return base::MappedReadOnlyRegion();
+  }
+
+  if (!GetData(region_mapping.mapping.memory(), data_size)) {
+    DLOG(ERROR) << "Failure getting metafile data into region";
+    return base::MappedReadOnlyRegion();
+  }
+
+  return region_mapping;
+}
+
 #if !defined(OS_ANDROID)
 bool Metafile::SaveTo(base::File* file) const {
   if (!file->IsValid())

printing/metafile.h

@@ -11,6 +11,7 @@
 
 #include "base/component_export.h"
 #include "base/containers/span.h"
+#include "base/memory/read_only_shared_memory_region.h"
 #include "build/build_config.h"
 #include "printing/mojom/print.mojom-forward.h"
 #include "printing/native_drawing_context.h"
@@ -66,6 +67,21 @@ class COMPONENT_EXPORT(PRINTING_METAFILE) MetafilePlayer {
   // called after the metafile is closed. Returns true if writing succeeded.
   virtual bool GetDataAsVector(std::vector<char>* buffer) const = 0;
 
+  // Generates a read-only shared memory region for the underlying data. This
+  // function should ONLY be called after the metafile is closed.  The returned
+  // region will be invalid if there is an error trying to generate the mapping.
+  virtual base::MappedReadOnlyRegion GetDataAsSharedMemoryRegion() const = 0;
+
+  // Determine if a copy of the data should be explicitly made before operating
+  // on metafile data.  For security purposes it is important to not operate
+  // directly on the metafile data shared across processes, but instead work on
+  // a local copy made of such data.  This query determines if such a copy needs
+  // to be made by the caller, since not all implementations are required to
+  // automatically do so.
+  // TODO(crbug.com/1135729)  Eliminate concern about making a copy when the
+  // shared memory can't be written by the sender.
+  virtual bool ShouldCopySharedMemoryRegionData() const = 0;
+
   // Identifies the type of encapsulated.
   virtual mojom::MetafileDataType GetDataType() const = 0;
 
@@ -144,8 +160,9 @@ class COMPONENT_EXPORT(PRINTING_METAFILE) Metafile : public MetafilePlayer {
                         const RECT* rect) const = 0;
 #endif  // OS_WIN
 
-  // MetfilePlayer
+  // MetfilePlayer implementation.
   bool GetDataAsVector(std::vector<char>* buffer) const override;
+  base::MappedReadOnlyRegion GetDataAsSharedMemoryRegion() const override;
 #if !defined(OS_ANDROID)
   bool SaveTo(base::File* file) const override;
 #endif  // !defined(OS_ANDROID)

printing/metafile_skia.cc

@@ -46,6 +46,10 @@
 
 namespace {
 
+// `InitFromData()` should make a copy of data for the safety of all operations
+// which would then operate upon that.
+constexpr bool kInitFromDataCopyData = true;
+
 bool WriteAssetToBuffer(const SkStreamAsset* asset, void* buffer, size_t size) {
   // Calling duplicate() keeps original asset state unchanged.
   std::unique_ptr<SkStreamAsset> assetCopy(asset->duplicate());
@@ -120,7 +124,7 @@ void MetafileSkia::UtilizeTypefaceContext(
 // MetafileSkia does.
 bool MetafileSkia::InitFromData(base::span<const uint8_t> data) {
   data_->data_stream = std::make_unique<SkMemoryStream>(
-      data.data(), data.size(), /*copy_data=*/true);
+      data.data(), data.size(), kInitFromDataCopyData);
   return true;
 }
 
@@ -266,6 +270,11 @@ bool MetafileSkia::GetData(void* dst_buffer, uint32_t dst_buffer_size) const {
                             base::checked_cast<size_t>(dst_buffer_size));
 }
 
+bool MetafileSkia::ShouldCopySharedMemoryRegionData() const {
+  // When `InitFromData()` copies the data, the caller doesn't have to.
+  return !kInitFromDataCopyData;
+}
+
 mojom::MetafileDataType MetafileSkia::GetDataType() const {
   return mojom::MetafileDataType::kPDF;
 }

printing/metafile_skia.h

@@ -55,6 +55,7 @@ class COMPONENT_EXPORT(PRINTING_METAFILE) MetafileSkia : public Metafile {
 
   uint32_t GetDataSize() const override;
   bool GetData(void* dst_buffer, uint32_t dst_buffer_size) const override;
+  bool ShouldCopySharedMemoryRegionData() const override;
   mojom::MetafileDataType GetDataType() const override;
 
   gfx::Rect GetPageBounds(unsigned int page_number) const override;

printing/pdf_metafile_cg_mac.cc

@@ -287,6 +287,11 @@ bool PdfMetafileCg::GetData(void* dst_buffer, uint32_t dst_buffer_size) const {
   return true;
 }
 
+bool PdfMetafileCg::ShouldCopySharedMemoryRegionData() const {
+  // Since `InitFromData()` copies the data, the caller doesn't have to.
+  return false;
+}
+
 mojom::MetafileDataType PdfMetafileCg::GetDataType() const {
   return mojom::MetafileDataType::kPDF;
 }

printing/pdf_metafile_cg_mac.h

@@ -35,6 +35,7 @@ class COMPONENT_EXPORT(PRINTING_METAFILE) PdfMetafileCg : public Metafile {
 
   uint32_t GetDataSize() const override;
   bool GetData(void* dst_buffer, uint32_t dst_buffer_size) const override;
+  bool ShouldCopySharedMemoryRegionData() const override;
   mojom::MetafileDataType GetDataType() const override;
 
   gfx::Rect GetPageBounds(unsigned int page_number) const override;