Commit 1f8eb50a authored by Matthew Cary's avatar Matthew Cary Committed by Commit Bot

base/memory: Add UnsafeSharedMemoryRegion::CreateFromHandle.

Create an UnsafeSharedMemoryRegion from a SharedMemoryHandle. This
creation method is only needed while transitioning to the new shared
memory API.

This enables in particular the media code to not add a dependency on
mojo, via WritableUnalignedMapping. This dependency has caused
problems, eg crbug.com/874074 and crbug.com/871429.

Bug: 795291
Change-Id: Ie0603e55afac9d853d371548a99475d763034fa5
Reviewed-on: https://chromium-review.googlesource.com/1183910
Commit-Queue: Matthew Cary <mattcary@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585886}
parent 74996623
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/gtest_prod_util.h" #include "base/gtest_prod_util.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/shared_memory_handle.h"
#include "base/unguessable_token.h" #include "base/unguessable_token.h"
#include "build/build_config.h" #include "build/build_config.h"
...@@ -130,6 +131,15 @@ class BASE_EXPORT PlatformSharedMemoryRegion { ...@@ -130,6 +131,15 @@ class BASE_EXPORT PlatformSharedMemoryRegion {
size_t size, size_t size,
const UnguessableToken& guid); const UnguessableToken& guid);
// As Take, above, but from a SharedMemoryHandle. This takes ownership of the
// handle. |mode| must be kUnsafe or kReadOnly; the latter must be used with a
// handle created with SharedMemoryHandle::GetReadOnlyHandle().
// TODO(crbug.com/795291): this should only be used while transitioning from
// the old shared memory API, and should be removed when done.
static PlatformSharedMemoryRegion TakeFromSharedMemoryHandle(
const SharedMemoryHandle& handle,
Mode mode);
// Default constructor initializes an invalid instance, i.e. an instance that // Default constructor initializes an invalid instance, i.e. an instance that
// doesn't wrap any valid platform handle. // doesn't wrap any valid platform handle.
PlatformSharedMemoryRegion(); PlatformSharedMemoryRegion();
......
...@@ -51,6 +51,20 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Take( ...@@ -51,6 +51,20 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Take(
return PlatformSharedMemoryRegion(std::move(fd), mode, size, guid); return PlatformSharedMemoryRegion(std::move(fd), mode, size, guid);
} }
// static
PlatformSharedMemoryRegion
PlatformSharedMemoryRegion::TakeFromSharedMemoryHandle(
const SharedMemoryHandle& handle,
Mode mode) {
CHECK((mode == Mode::kReadOnly && handle.IsReadOnly()) ||
(mode == Mode::kUnsafe && !handle.IsReadOnly()));
if (!handle.IsValid())
return {};
return Take(ScopedFD(handle.GetHandle()), mode, handle.GetSize(),
handle.GetGUID());
}
int PlatformSharedMemoryRegion::GetPlatformHandle() const { int PlatformSharedMemoryRegion::GetPlatformHandle() const {
return handle_.get(); return handle_.get();
} }
......
...@@ -41,6 +41,19 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Take( ...@@ -41,6 +41,19 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Take(
return PlatformSharedMemoryRegion(std::move(handle), mode, size, guid); return PlatformSharedMemoryRegion(std::move(handle), mode, size, guid);
} }
// static
PlatformSharedMemoryRegion
PlatformSharedMemoryRegion::TakeFromSharedMemoryHandle(
const SharedMemoryHandle& handle,
Mode mode) {
CHECK(mode == Mode::kReadOnly || mode == Mode::kUnsafe);
if (!handle.IsValid())
return {};
return Take(zx::vmo(handle.GetHandle()), mode, handle.GetSize(),
handle.GetGUID());
}
zx_handle_t PlatformSharedMemoryRegion::GetPlatformHandle() const { zx_handle_t PlatformSharedMemoryRegion::GetPlatformHandle() const {
return handle_.get(); return handle_.get();
} }
......
...@@ -39,6 +39,20 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Take( ...@@ -39,6 +39,20 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Take(
return PlatformSharedMemoryRegion(std::move(handle), mode, size, guid); return PlatformSharedMemoryRegion(std::move(handle), mode, size, guid);
} }
// static
PlatformSharedMemoryRegion
PlatformSharedMemoryRegion::TakeFromSharedMemoryHandle(
const SharedMemoryHandle& handle,
Mode mode) {
CHECK(mode == Mode::kReadOnly || mode == Mode::kUnsafe);
CHECK(handle.GetType() == SharedMemoryHandle::MACH);
if (!handle.IsValid())
return {};
return Take(base::mac::ScopedMachSendRight(handle.GetMemoryObject()), mode,
handle.GetSize(), handle.GetGUID());
}
mach_port_t PlatformSharedMemoryRegion::GetPlatformHandle() const { mach_port_t PlatformSharedMemoryRegion::GetPlatformHandle() const {
return handle_.get(); return handle_.get();
} }
......
...@@ -109,6 +109,20 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Take( ...@@ -109,6 +109,20 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Take(
return PlatformSharedMemoryRegion(std::move(handle), mode, size, guid); return PlatformSharedMemoryRegion(std::move(handle), mode, size, guid);
} }
// static
PlatformSharedMemoryRegion
PlatformSharedMemoryRegion::TakeFromSharedMemoryHandle(
const SharedMemoryHandle& handle,
Mode mode) {
CHECK(mode == Mode::kReadOnly || mode == Mode::kUnsafe);
if (!handle.IsValid())
return {};
return Take(
base::subtle::ScopedFDPair(ScopedFD(handle.GetHandle()), ScopedFD()),
mode, handle.GetSize(), handle.GetGUID());
}
FDPair PlatformSharedMemoryRegion::GetPlatformHandle() const { FDPair PlatformSharedMemoryRegion::GetPlatformHandle() const {
return handle_.get(); return handle_.get();
} }
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "base/memory/platform_shared_memory_region.h" #include "base/memory/platform_shared_memory_region.h"
#include "base/memory/shared_memory.h"
#include "base/memory/shared_memory_mapping.h" #include "base/memory/shared_memory_mapping.h"
#include "base/process/process_metrics.h" #include "base/process/process_metrics.h"
#include "base/sys_info.h" #include "base/sys_info.h"
...@@ -343,5 +344,66 @@ TEST_F(PlatformSharedMemoryRegionTest, UnsafeRegionConvertToUnsafeDeathTest) { ...@@ -343,5 +344,66 @@ TEST_F(PlatformSharedMemoryRegionTest, UnsafeRegionConvertToUnsafeDeathTest) {
EXPECT_DEATH_IF_SUPPORTED(region.ConvertToUnsafe(), kErrorRegex); EXPECT_DEATH_IF_SUPPORTED(region.ConvertToUnsafe(), kErrorRegex);
} }
// Check that taking from a SharedMemoryHandle works.
TEST_F(PlatformSharedMemoryRegionTest, TakeFromSharedMemoryHandle) {
SharedMemory shm;
auto region = PlatformSharedMemoryRegion::TakeFromSharedMemoryHandle(
shm.TakeHandle(), PlatformSharedMemoryRegion::Mode::kUnsafe);
ASSERT_FALSE(region.IsValid());
shm.CreateAndMapAnonymous(10);
region = PlatformSharedMemoryRegion::TakeFromSharedMemoryHandle(
shm.TakeHandle(), PlatformSharedMemoryRegion::Mode::kUnsafe);
ASSERT_TRUE(region.IsValid());
#if !(defined(OS_MACOSX) && !defined(OS_IOS))
// Note that it's not possible on all platforms for TakeFromSharedMemoryHandle
// to conveniently check if the SharedMemoryHandle is readonly or
// not. Therefore it is actually possible to get an kUnsafe
// PlatformSharedMemoryRegion from a readonly handle on some platforms.
SharedMemoryCreateOptions options;
options.size = 10;
options.share_read_only = true;
shm.Create(options);
EXPECT_DEATH_IF_SUPPORTED(
PlatformSharedMemoryRegion::TakeFromSharedMemoryHandle(
shm.GetReadOnlyHandle(), PlatformSharedMemoryRegion::Mode::kUnsafe),
"");
#endif // !(defined(OS_MACOSX) && !defined(OS_IOS))
}
// Check that taking from a readonly SharedMemoryHandle works.
TEST_F(PlatformSharedMemoryRegionTest, TakeFromReadOnlySharedMemoryHandle) {
SharedMemory shm;
// Note that getting a read-only handle from an unmapped SharedMemory will
// fail, so the invalid region case cannot be tested.
SharedMemoryCreateOptions options;
options.size = 10;
options.share_read_only = true;
shm.Create(options);
auto readonly_handle = shm.GetReadOnlyHandle();
#if defined(OS_ANDROID)
readonly_handle.SetRegionReadOnly();
#endif
auto region = PlatformSharedMemoryRegion::TakeFromSharedMemoryHandle(
readonly_handle, PlatformSharedMemoryRegion::Mode::kReadOnly);
ASSERT_TRUE(region.IsValid());
}
// Check that taking from a SharedMemoryHandle in writable mode fails.
TEST_F(PlatformSharedMemoryRegionTest, WritableTakeFromSharedMemoryHandle) {
SharedMemory shm;
EXPECT_DEATH_IF_SUPPORTED(
PlatformSharedMemoryRegion::TakeFromSharedMemoryHandle(
shm.TakeHandle(), PlatformSharedMemoryRegion::Mode::kWritable),
"");
shm.CreateAndMapAnonymous(10);
EXPECT_DEATH_IF_SUPPORTED(
PlatformSharedMemoryRegion::TakeFromSharedMemoryHandle(
shm.TakeHandle(), PlatformSharedMemoryRegion::Mode::kWritable),
"");
}
} // namespace subtle } // namespace subtle
} // namespace base } // namespace base
...@@ -157,6 +157,19 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Take( ...@@ -157,6 +157,19 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Take(
return PlatformSharedMemoryRegion(std::move(handle), mode, size, guid); return PlatformSharedMemoryRegion(std::move(handle), mode, size, guid);
} }
// static
PlatformSharedMemoryRegion
PlatformSharedMemoryRegion::TakeFromSharedMemoryHandle(
const SharedMemoryHandle& handle,
Mode mode) {
CHECK(mode == Mode::kReadOnly || mode == Mode::kUnsafe);
if (!handle.IsValid())
return {};
return Take(base::win::ScopedHandle(handle.GetHandle()), mode,
handle.GetSize(), handle.GetGUID());
}
HANDLE PlatformSharedMemoryRegion::GetPlatformHandle() const { HANDLE PlatformSharedMemoryRegion::GetPlatformHandle() const {
return handle_.Get(); return handle_.Get();
} }
......
...@@ -122,6 +122,8 @@ class BASE_EXPORT SharedMemoryHandle { ...@@ -122,6 +122,8 @@ class BASE_EXPORT SharedMemoryHandle {
mach_vm_size_t size, mach_vm_size_t size,
const base::UnguessableToken& guid); const base::UnguessableToken& guid);
Type GetType() const { return type_; }
// Exposed so that the SharedMemoryHandle can be transported between // Exposed so that the SharedMemoryHandle can be transported between
// processes. // processes.
mach_port_t GetMemoryObject() const; mach_port_t GetMemoryObject() const;
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "base/memory/platform_shared_memory_region.h" #include "base/memory/platform_shared_memory_region.h"
#include "base/memory/read_only_shared_memory_region.h" #include "base/memory/read_only_shared_memory_region.h"
#include "base/memory/shared_memory.h"
#include "base/memory/unsafe_shared_memory_region.h" #include "base/memory/unsafe_shared_memory_region.h"
#include "base/memory/writable_shared_memory_region.h" #include "base/memory/writable_shared_memory_region.h"
#include "base/sys_info.h" #include "base/sys_info.h"
...@@ -277,4 +278,17 @@ TEST_F(ReadOnlySharedMemoryRegionTest, ...@@ -277,4 +278,17 @@ TEST_F(ReadOnlySharedMemoryRegionTest,
EXPECT_DEATH_IF_SUPPORTED(memset(memory_ptr, 'G', kRegionSize), ""); EXPECT_DEATH_IF_SUPPORTED(memset(memory_ptr, 'G', kRegionSize), "");
} }
class UnsafeSharedMemoryRegionTest : public ::testing::Test {};
TEST_F(UnsafeSharedMemoryRegionTest, CreateFromHandleTest) {
SharedMemory shm;
auto region = UnsafeSharedMemoryRegion::CreateFromHandle(shm.TakeHandle());
ASSERT_FALSE(region.IsValid());
shm.CreateAndMapAnonymous(10);
region = UnsafeSharedMemoryRegion::CreateFromHandle(shm.TakeHandle());
ASSERT_TRUE(region.IsValid());
}
} // namespace base } // namespace base
...@@ -18,6 +18,20 @@ UnsafeSharedMemoryRegion UnsafeSharedMemoryRegion::Create(size_t size) { ...@@ -18,6 +18,20 @@ UnsafeSharedMemoryRegion UnsafeSharedMemoryRegion::Create(size_t size) {
return UnsafeSharedMemoryRegion(std::move(handle)); return UnsafeSharedMemoryRegion(std::move(handle));
} }
// static
UnsafeSharedMemoryRegion UnsafeSharedMemoryRegion::CreateFromHandle(
const SharedMemoryHandle& handle) {
if (!handle.IsValid())
return UnsafeSharedMemoryRegion();
auto platform_region =
subtle::PlatformSharedMemoryRegion::TakeFromSharedMemoryHandle(
handle, subtle::PlatformSharedMemoryRegion::Mode::kUnsafe);
if (!platform_region.IsValid()) {
return UnsafeSharedMemoryRegion();
}
return Deserialize(std::move(platform_region));
}
// static // static
UnsafeSharedMemoryRegion UnsafeSharedMemoryRegion::Deserialize( UnsafeSharedMemoryRegion UnsafeSharedMemoryRegion::Deserialize(
subtle::PlatformSharedMemoryRegion handle) { subtle::PlatformSharedMemoryRegion handle) {
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/gtest_prod_util.h" #include "base/gtest_prod_util.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/platform_shared_memory_region.h" #include "base/memory/platform_shared_memory_region.h"
#include "base/memory/shared_memory_handle.h"
#include "base/memory/shared_memory_mapping.h" #include "base/memory/shared_memory_mapping.h"
namespace base { namespace base {
...@@ -39,6 +40,13 @@ class BASE_EXPORT UnsafeSharedMemoryRegion { ...@@ -39,6 +40,13 @@ class BASE_EXPORT UnsafeSharedMemoryRegion {
// region from a an unprivileged process where a broker must be used. // region from a an unprivileged process where a broker must be used.
static UnsafeSharedMemoryRegion Create(size_t size); static UnsafeSharedMemoryRegion Create(size_t size);
// Creates a new UnsafeSharedMemoryRegion from a SharedMemoryHandle. This
// consumes the handle, which should not be used again.
// TODO(crbug.com/795291): this should only be used while transitioning from
// the old shared memory API, and should be removed when done.
static UnsafeSharedMemoryRegion CreateFromHandle(
const base::SharedMemoryHandle& handle);
// Returns an UnsafeSharedMemoryRegion built from a platform-specific handle // Returns an UnsafeSharedMemoryRegion built from a platform-specific handle
// that was taken from another UnsafeSharedMemoryRegion instance. Returns an // that was taken from another UnsafeSharedMemoryRegion instance. Returns an
// invalid region iff the |handle| is invalid. CHECK-fails if the |handle| // invalid region iff the |handle| is invalid. CHECK-fails if the |handle|
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment