media/base: Add new UnalignedWritableMapping type.

Where the old UnalignedSharedMemory type is created from a legacy
SharedMemoryHandle, this new type is created from a new shared memory
API instance of UnsafeSharedMemoryRegion. This changes both the
ownership semantics and mapping details from UnalignedSharedMemory: the
new UnalignedWritableMapping does not own the region backing the
shared memory, and only owns the mapping itself. Because of this, the
region is mapped upon construction rather than with a later MapAt
call.

These changes do not seem to affect behavior in a practical manner.

This change also removes the read-only bit. This is no longer
meaningful, as only the mapping is owned; in addition the original use
of this variable was not correct, as explained in detail in
https://goo.gl/HmBYy6. (TL;DR: on some platforms, being read-only is a
characteristic of the underlying OS region, and cannot be set in some
processes without affecting the protection in all processes. As this
behavior is incompatible with usage, this means that sometimes shared
memory which has been declared read-only is not, in fact, read-only, and
could be exploited by rouge processes).

Finally, a the new UnalignedWritableMapping contains a shim
constructor which takes a legacy SharedMemoryHandle. The ownership
semantics are the same a the region-created version, that is, the handle
is *not* owned by the UnalignedWritableMapping instance.

This shim allows us to convert all users of this class one-by-one, and
confirm that the new ownership semantics are correctly handled. After
this is done, the BitstreamBuffer class, which is at the heart of the
IPCs involved, can be converted to the new API without needing to change
ownership semantics at the same time.

See the bug for an overview of the entire process.

Bug: 849207
Change-Id: Ibe4bbb48f7bea3fb31728bf98133ede0228b9801
Reviewed-on: https://chromium-review.googlesource.com/1116703Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Alexandr Ilin <alexilin@chromium.org>
Commit-Queue: Matthew Cary <mattcary@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570930}

media/DEPS

@@ -8,6 +8,7 @@ include_rules = [
   "+gpu",
   "+jni",
   "+mojo/public/cpp/bindings/callback_helpers.h",
+  "+mojo/public/cpp/system/platform_handle.h",
   "+services/ui/public/cpp/gpu/context_provider_command_buffer.h",
   "+skia/ext",
   "+third_party/ffmpeg",

media/base/unaligned_shared_memory.cc

@@ -8,6 +8,7 @@
 
 #include "base/logging.h"
 #include "base/sys_info.h"
+#include "mojo/public/cpp/system/platform_handle.h"
 
 namespace media {
 
@@ -61,4 +62,71 @@ void* UnalignedSharedMemory::memory() const {
   return static_cast<uint8_t*>(shm_.memory()) + misalignment_;
 }
 
+WritableUnalignedMapping::WritableUnalignedMapping(
+    const base::UnsafeSharedMemoryRegion& region,
+    size_t size,
+    off_t offset)
+    : size_(size), misalignment_(0) {
+  if (!region.IsValid()) {
+    DLOG(ERROR) << "Invalid region";
+    return;
+  }
+
+  if (offset < 0) {
+    DLOG(ERROR) << "Invalid offset";
+    return;
+  }
+
+  /* |   |   |   |   |   |  shm pages
+   *       |                offset (may exceed max size_t)
+   *       |-----------|    size
+   *     |-|                misalignment
+   *     |                  adjusted offset
+   *     |-------------|    requested mapping
+   */
+  // Note: result of % computation may be off_t or size_t, depending on the
+  // relative ranks of those types. In any case we assume that
+  // VMAllocationGranularity() fits in both types, so the final result does too.
+  misalignment_ = offset % base::SysInfo::VMAllocationGranularity();
+
+  // Above this |size_|, |size_| + |misalignment| overflows.
+  size_t max_size = std::numeric_limits<size_t>::max() - misalignment_;
+  if (size_ > max_size) {
+    DLOG(ERROR) << "Invalid size";
+    return;
+  }
+
+  off_t adjusted_offset = offset - static_cast<off_t>(misalignment_);
+  mapping_ = region.MapAt(adjusted_offset, size_ + misalignment_);
+  if (!mapping_.IsValid()) {
+    DLOG(ERROR) << "Failed to map shared memory " << adjusted_offset << "("
+                << offset << ")"
+                << "@" << size << "/\\" << misalignment_ << " on "
+                << region.GetSize();
+
+    return;
+  }
+}
+
+WritableUnalignedMapping::WritableUnalignedMapping(
+    const base::SharedMemoryHandle& handle,
+    size_t size,
+    off_t offset)
+    : WritableUnalignedMapping(
+          mojo::UnwrapUnsafeSharedMemoryRegion(mojo::WrapSharedMemoryHandle(
+              handle,
+              size,
+              mojo::UnwrappedSharedMemoryHandleProtection::kReadWrite)),
+          size,
+          offset) {}
+
+WritableUnalignedMapping::~WritableUnalignedMapping() = default;
+
+void* WritableUnalignedMapping::memory() const {
+  if (!IsValid()) {
+    return nullptr;
+  }
+  return static_cast<uint8_t*>(mapping_.memory()) + misalignment_;
+}
+
 }  // namespace media

media/base/unaligned_shared_memory.h

@@ -9,15 +9,19 @@
 
 #include "base/macros.h"
 #include "base/memory/shared_memory.h"
+#include "base/memory/shared_memory_mapping.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "media/base/media_export.h"
 
 namespace media {
 
 // Wrapper over base::SharedMemory that can be mapped at unaligned offsets.
+// DEPRECATED! See https://crbug.com/795291.
 class MEDIA_EXPORT UnalignedSharedMemory {
  public:
   // Creates an |UnalignedSharedMemory| instance from a
-  // |SharedMemoryHandle|. |size| sets the maximum size that may be mapped.
+  // |SharedMemoryHandle|. |size| sets the maximum size that may be mapped. This
+  // instance will own the handle.
   UnalignedSharedMemory(const base::SharedMemoryHandle& handle,
                         size_t size,
                         bool read_only);
@@ -42,6 +46,47 @@ class MEDIA_EXPORT UnalignedSharedMemory {
   DISALLOW_COPY_AND_ASSIGN(UnalignedSharedMemory);
 };
 
+// Wrapper over base::WritableSharedMemoryMapping that is mapped at unaligned
+// offsets.
+class MEDIA_EXPORT WritableUnalignedMapping {
+ public:
+  // Creates an |UnalignedSharedMemory| instance from a
+  // |UnsafeSharedMemoryRegion|. |size| sets the maximum size that may be mapped
+  // within |region| and |offset| is the offset that will be mapped. |region| is
+  // not retained and is used only in the constructor.
+  WritableUnalignedMapping(const base::UnsafeSharedMemoryRegion& region,
+                           size_t size,
+                           off_t offset);
+
+  // As above, but creates from a handle. This region will not own the handle.
+  // DEPRECATED: this should be used only for the legacy shared memory
+  // conversion project, see https://crbug.com/795291.
+  WritableUnalignedMapping(const base::SharedMemoryHandle& handle,
+                           size_t size,
+                           off_t offset);
+
+  ~WritableUnalignedMapping();
+
+  size_t size() const { return size_; }
+  void* memory() const;
+
+  // True if the mapping backing the memory is valid.
+  bool IsValid() const { return mapping_.IsValid(); }
+
+ private:
+  base::WritableSharedMemoryMapping mapping_;
+
+  // The size of the region associated with |mapping_|.
+  size_t size_;
+
+  // Difference between actual offset within |mapping_| where data has been
+  // mapped and requested offset; strictly less than
+  // base::SysInfo::VMAllocationGranularity().
+  size_t misalignment_;
+
+  DISALLOW_COPY_AND_ASSIGN(WritableUnalignedMapping);
+};
+
 }  // namespace media
 
 #endif  // MEDIA_BASE_UNALIGNED_SHARED_MEMORY_H_

media/base/unaligned_shared_memory_unittest.cc

@@ -32,6 +32,14 @@ base::SharedMemoryHandle CreateHandle(const uint8_t* data, size_t size) {
   return shm.TakeHandle();
 }
 
+base::UnsafeSharedMemoryRegion CreateRegion(const uint8_t* data, size_t size) {
+  auto region = base::UnsafeSharedMemoryRegion::Create(size);
+  auto mapping = region.Map();
+  EXPECT_TRUE(mapping.IsValid());
+  memcpy(mapping.memory(), data, size);
+  return region;
+}
+
 }  // namespace
 
 TEST(UnalignedSharedMemoryTest, CreateAndDestroy) {
@@ -83,4 +91,61 @@ TEST(UnalignedSharedMemoryTest, UnmappedIsNullptr) {
   ASSERT_EQ(shm.memory(), nullptr);
 }
 
+TEST(WritableUnalignedMappingTest, CreateAndDestroy) {
+  auto region = CreateRegion(kData, kDataSize);
+  WritableUnalignedMapping shm(region, kDataSize, 0);
+  EXPECT_TRUE(shm.IsValid());
+}
+
+TEST(WritableUnalignedMappingTest, CreateAndDestroy_InvalidHandle) {
+  base::SharedMemoryHandle handle;
+  WritableUnalignedMapping shm(handle, kDataSize, 0);
+  EXPECT_FALSE(shm.IsValid());
+}
+
+TEST(WritableUnalignedMappingTest, CreateAndDestroyHandle) {
+  auto handle = CreateHandle(kData, kDataSize);
+  WritableUnalignedMapping shm(handle, kDataSize, 0);
+  EXPECT_TRUE(shm.IsValid());
+}
+
+TEST(WritableUnalignedMappingTest, CreateAndDestroy_InvalidRegion) {
+  base::UnsafeSharedMemoryRegion region;
+  WritableUnalignedMapping shm(region, kDataSize, 0);
+  EXPECT_FALSE(shm.IsValid());
+}
+
+TEST(WritableUnalignedMappingTest, Map) {
+  auto region = CreateRegion(kData, kDataSize);
+  WritableUnalignedMapping shm(region, kDataSize, 0);
+  ASSERT_TRUE(shm.IsValid());
+  EXPECT_EQ(0, memcmp(shm.memory(), kData, kDataSize));
+}
+
+TEST(WritableUnalignedMappingTest, Map_Unaligned) {
+  auto region = CreateRegion(kUnalignedData, kUnalignedDataSize);
+  WritableUnalignedMapping shm(region, kDataSize, kUnalignedOffset);
+  ASSERT_TRUE(shm.IsValid());
+  EXPECT_EQ(0, memcmp(shm.memory(), kData, kDataSize));
+}
+
+TEST(WritableUnalignedMappingTest, Map_InvalidRegion) {
+  base::UnsafeSharedMemoryRegion region;
+  WritableUnalignedMapping shm(region, kDataSize, 0);
+  ASSERT_FALSE(shm.IsValid());
+  EXPECT_EQ(shm.memory(), nullptr);
+}
+
+TEST(WritableUnalignedMappingTest, Map_NegativeOffset) {
+  auto region = CreateRegion(kData, kDataSize);
+  WritableUnalignedMapping shm(region, kDataSize, -1);
+  ASSERT_FALSE(shm.IsValid());
+}
+
+TEST(WritableUnalignedMappingTest, Map_SizeOverflow) {
+  auto region = CreateRegion(kData, kDataSize);
+  WritableUnalignedMapping shm(region, std::numeric_limits<size_t>::max(), 1);
+  ASSERT_FALSE(shm.IsValid());
+}
+
 }  // namespace media