From ee191a43debecee40db66052a2de1621f50a2c49 Mon Sep 17 00:00:00 2001 From: amistry Date: Wed, 6 Apr 2016 19:54:07 -0700 Subject: [PATCH] Support read-only duplicates of Mojo shared buffers. BUG=556587 Review URL: https://codereview.chromium.org/1689053003 Cr-Commit-Position: refs/heads/master@{#385634} --- mojo/edk/embedder/embedder.h | 4 - mojo/edk/embedder/platform_shared_buffer.cc | 41 ++++++-- mojo/edk/embedder/platform_shared_buffer.h | 19 ++-- mojo/edk/system/broker_posix.cc | 3 +- mojo/edk/system/core.cc | 3 +- .../system/data_pipe_consumer_dispatcher.cc | 1 + .../system/data_pipe_producer_dispatcher.cc | 1 + mojo/edk/system/shared_buffer_dispatcher.cc | 31 +++++- .../shared_buffer_dispatcher_unittest.cc | 6 ++ mojo/edk/system/shared_buffer_unittest.cc | 96 ++++++++++++++++++- mojo/edk/test/mojo_test_base.cc | 10 +- mojo/edk/test/mojo_test_base.h | 2 +- mojo/public/c/system/buffer.h | 5 + 13 files changed, 189 insertions(+), 33 deletions(-) diff --git a/mojo/edk/embedder/embedder.h b/mojo/edk/embedder/embedder.h index 3eadc36495c3ce..6a9b30525c3c9c 100644 --- a/mojo/edk/embedder/embedder.h +++ b/mojo/edk/embedder/embedder.h @@ -94,8 +94,6 @@ PassWrappedPlatformHandle(MojoHandle platform_handle_wrapper_handle, // |read_only| is whether the handle is a read-only handle to shared memory. // This |MojoHandle| is a Mojo shared buffer and can be manipulated using the // shared buffer functions and transferred over a message pipe. -// TODO(crbug.com/556587): Support read-only handles. Currently, |read_only| -// must be false. MOJO_SYSTEM_IMPL_EXPORT MojoResult CreateSharedBufferWrapper(base::SharedMemoryHandle shared_memory_handle, size_t num_bytes, @@ -109,8 +107,6 @@ CreateSharedBufferWrapper(base::SharedMemoryHandle shared_memory_handle, // Note: The value of |shared_memory_handle| may be // base::SharedMemory::NULLHandle(), even if this function returns success. // Callers should perform appropriate checks. -// TODO(crbug.com/556587): Support read-only handles. Currently, |read_only| -// will always return |false|. MOJO_SYSTEM_IMPL_EXPORT MojoResult PassSharedMemoryHandle(MojoHandle mojo_handle, base::SharedMemoryHandle* shared_memory_handle, diff --git a/mojo/edk/embedder/platform_shared_buffer.cc b/mojo/edk/embedder/platform_shared_buffer.cc index 614b19c0c0d488..a29a7abd76152e 100644 --- a/mojo/edk/embedder/platform_shared_buffer.cc +++ b/mojo/edk/embedder/platform_shared_buffer.cc @@ -44,7 +44,7 @@ ScopedPlatformHandle SharedMemoryToPlatformHandle( PlatformSharedBuffer* PlatformSharedBuffer::Create(size_t num_bytes) { DCHECK_GT(num_bytes, 0u); - PlatformSharedBuffer* rv = new PlatformSharedBuffer(num_bytes); + PlatformSharedBuffer* rv = new PlatformSharedBuffer(num_bytes, false); if (!rv->Init()) { // We can't just delete it directly, due to the "in destructor" (debug) // check. @@ -58,10 +58,11 @@ PlatformSharedBuffer* PlatformSharedBuffer::Create(size_t num_bytes) { // static PlatformSharedBuffer* PlatformSharedBuffer::CreateFromPlatformHandle( size_t num_bytes, + bool read_only, ScopedPlatformHandle platform_handle) { DCHECK_GT(num_bytes, 0u); - PlatformSharedBuffer* rv = new PlatformSharedBuffer(num_bytes); + PlatformSharedBuffer* rv = new PlatformSharedBuffer(num_bytes, read_only); if (!rv->InitFromPlatformHandle(std::move(platform_handle))) { // We can't just delete it directly, due to the "in destructor" (debug) // check. @@ -78,9 +79,8 @@ PlatformSharedBuffer* PlatformSharedBuffer::CreateFromSharedMemoryHandle( bool read_only, base::SharedMemoryHandle handle) { DCHECK_GT(num_bytes, 0u); - DCHECK(!read_only); - PlatformSharedBuffer* rv = new PlatformSharedBuffer(num_bytes); + PlatformSharedBuffer* rv = new PlatformSharedBuffer(num_bytes, read_only); rv->InitFromSharedMemoryHandle(handle); return rv; @@ -90,6 +90,10 @@ size_t PlatformSharedBuffer::GetNumBytes() const { return num_bytes_; } +bool PlatformSharedBuffer::IsReadOnly() const { + return read_only_; +} + scoped_ptr PlatformSharedBuffer::Map( size_t offset, size_t length) { @@ -125,7 +129,7 @@ scoped_ptr PlatformSharedBuffer::MapNoCheck( return nullptr; scoped_ptr mapping( - new PlatformSharedBufferMapping(handle, offset, length)); + new PlatformSharedBufferMapping(handle, read_only_, offset, length)); if (mapping->Map()) return make_scoped_ptr(mapping.release()); @@ -164,16 +168,34 @@ base::SharedMemoryHandle PlatformSharedBuffer::DuplicateSharedMemoryHandle() { return base::SharedMemory::DuplicateHandle(shared_memory_->handle()); } -PlatformSharedBuffer::PlatformSharedBuffer(size_t num_bytes) - : num_bytes_(num_bytes) {} +PlatformSharedBuffer* PlatformSharedBuffer::CreateReadOnlyDuplicate() { + DCHECK(shared_memory_); + base::SharedMemoryHandle handle; + bool success; + { + base::AutoLock locker(lock_); + success = shared_memory_->ShareReadOnlyToProcess( + base::GetCurrentProcessHandle(), &handle); + } + if (!success || handle == base::SharedMemory::NULLHandle()) + return nullptr; + + return CreateFromSharedMemoryHandle(num_bytes_, true, handle); +} + +PlatformSharedBuffer::PlatformSharedBuffer(size_t num_bytes, bool read_only) + : num_bytes_(num_bytes), read_only_(read_only) {} PlatformSharedBuffer::~PlatformSharedBuffer() {} bool PlatformSharedBuffer::Init() { DCHECK(!shared_memory_); + DCHECK(!read_only_); base::SharedMemoryCreateOptions options; options.size = num_bytes_; + // By default, we can share as read-only. + options.share_read_only = true; #if defined(OS_MACOSX) && !defined(OS_IOS) options.type = base::SharedMemoryHandle::MACH; #endif @@ -201,7 +223,7 @@ bool PlatformSharedBuffer::InitFromPlatformHandle( base::SharedMemoryHandle handle(platform_handle.release().handle, false); #endif - shared_memory_.reset(new base::SharedMemory(handle, false)); + shared_memory_.reset(new base::SharedMemory(handle, read_only_)); return true; } @@ -209,8 +231,7 @@ void PlatformSharedBuffer::InitFromSharedMemoryHandle( base::SharedMemoryHandle handle) { DCHECK(!shared_memory_); - // TODO(crbug.com/556587): Support read-only handles. - shared_memory_.reset(new base::SharedMemory(handle, false)); + shared_memory_.reset(new base::SharedMemory(handle, read_only_)); } PlatformSharedBufferMapping::~PlatformSharedBufferMapping() { diff --git a/mojo/edk/embedder/platform_shared_buffer.h b/mojo/edk/embedder/platform_shared_buffer.h index dfe9165140345e..1cfcd3a9aca860 100644 --- a/mojo/edk/embedder/platform_shared_buffer.h +++ b/mojo/edk/embedder/platform_shared_buffer.h @@ -31,9 +31,6 @@ class PlatformSharedBufferMapping; // - Sizes/offsets (of the shared memory and mappings) are arbitrary, and not // restricted by page size. However, more memory may actually be mapped than // requested. -// -// It currently does NOT support the following: -// - Sharing read-only. (This will probably eventually be supported.) class MOJO_SYSTEM_IMPL_EXPORT PlatformSharedBuffer : public base::RefCountedThreadSafe { public: @@ -45,6 +42,7 @@ class MOJO_SYSTEM_IMPL_EXPORT PlatformSharedBuffer // handle |platform_handle|. Returns null on failure. static PlatformSharedBuffer* CreateFromPlatformHandle( size_t num_bytes, + bool read_only, ScopedPlatformHandle platform_handle); // Creates a shared buffer of size |num_bytes| from the existing shared memory @@ -57,6 +55,9 @@ class MOJO_SYSTEM_IMPL_EXPORT PlatformSharedBuffer // Gets the size of shared buffer (in number of bytes). size_t GetNumBytes() const; + // Returns whether this shared buffer is read-only. + bool IsReadOnly() const; + // Maps (some) of the shared buffer into memory; [|offset|, |offset + length|] // must be contained in [0, |num_bytes|], and |length| must be at least 1. // Returns null on failure. @@ -71,7 +72,6 @@ class MOJO_SYSTEM_IMPL_EXPORT PlatformSharedBuffer size_t length); // Duplicates the underlying platform handle and passes it to the caller. - // TODO(vtl): On POSIX, we'll need two FDs to support sharing read-only. ScopedPlatformHandle DuplicatePlatformHandle(); // Duplicates the underlying shared memory handle and passes it to the caller. @@ -83,10 +83,15 @@ class MOJO_SYSTEM_IMPL_EXPORT PlatformSharedBuffer // be disposed of. ScopedPlatformHandle PassPlatformHandle(); + // Create and return a read-only duplicate of this shared buffer. If this + // shared buffer isn't capable of returning a read-only duplicate, then + // nullptr will be returned. + PlatformSharedBuffer* CreateReadOnlyDuplicate(); + private: friend class base::RefCountedThreadSafe; - explicit PlatformSharedBuffer(size_t num_bytes); + PlatformSharedBuffer(size_t num_bytes, bool read_only); ~PlatformSharedBuffer(); // This is called by |Create()| before this object is given to anyone. @@ -100,6 +105,7 @@ class MOJO_SYSTEM_IMPL_EXPORT PlatformSharedBuffer void InitFromSharedMemoryHandle(base::SharedMemoryHandle handle); const size_t num_bytes_; + const bool read_only_; base::Lock lock_; scoped_ptr shared_memory_; @@ -126,12 +132,13 @@ class MOJO_SYSTEM_IMPL_EXPORT PlatformSharedBufferMapping { friend class PlatformSharedBuffer; PlatformSharedBufferMapping(base::SharedMemoryHandle handle, + bool read_only, size_t offset, size_t length) : offset_(offset), length_(length), base_(nullptr), - shared_memory_(handle, false) {} + shared_memory_(handle, read_only) {} bool Map(); void Unmap(); diff --git a/mojo/edk/system/broker_posix.cc b/mojo/edk/system/broker_posix.cc index 91b9cb5a998c2c..3cbb953f1cb1ae 100644 --- a/mojo/edk/system/broker_posix.cc +++ b/mojo/edk/system/broker_posix.cc @@ -112,7 +112,8 @@ scoped_refptr Broker::GetSharedBuffer(size_t num_bytes) { BrokerMessageType::BUFFER_RESPONSE, 1, &incoming_platform_handles)) { return PlatformSharedBuffer::CreateFromPlatformHandle( - num_bytes, ScopedPlatformHandle(incoming_platform_handles.front())); + num_bytes, false /* read_only */, + ScopedPlatformHandle(incoming_platform_handles.front())); } return nullptr; diff --git a/mojo/edk/system/core.cc b/mojo/edk/system/core.cc index ca6105de0f2494..b3602d7c5f4a16 100644 --- a/mojo/edk/system/core.cc +++ b/mojo/edk/system/core.cc @@ -160,7 +160,6 @@ MojoResult Core::CreateSharedBufferWrapper( bool read_only, MojoHandle* mojo_wrapper_handle) { DCHECK(num_bytes); - CHECK(!read_only); scoped_refptr platform_buffer = PlatformSharedBuffer::CreateFromSharedMemoryHandle(num_bytes, read_only, shared_memory_handle); @@ -214,7 +213,7 @@ MojoResult Core::PassSharedMemoryHandle( if (num_bytes) *num_bytes = platform_shared_buffer->GetNumBytes(); if (read_only) - *read_only = false; + *read_only = platform_shared_buffer->IsReadOnly(); *shared_memory_handle = platform_shared_buffer->DuplicateSharedMemoryHandle(); shm_dispatcher->Close(); diff --git a/mojo/edk/system/data_pipe_consumer_dispatcher.cc b/mojo/edk/system/data_pipe_consumer_dispatcher.cc index f978f9498e99f9..7bf0b0177acdbd 100644 --- a/mojo/edk/system/data_pipe_consumer_dispatcher.cc +++ b/mojo/edk/system/data_pipe_consumer_dispatcher.cc @@ -403,6 +403,7 @@ DataPipeConsumerDispatcher::Deserialize(const void* data, scoped_refptr ring_buffer = PlatformSharedBuffer::CreateFromPlatformHandle( state->options.capacity_num_bytes, + false /* read_only */, ScopedPlatformHandle(buffer_handle)); if (!ring_buffer) { DLOG(ERROR) << "Failed to deserialize shared buffer handle."; diff --git a/mojo/edk/system/data_pipe_producer_dispatcher.cc b/mojo/edk/system/data_pipe_producer_dispatcher.cc index d8d8abf1791c0e..d3b2530035889b 100644 --- a/mojo/edk/system/data_pipe_producer_dispatcher.cc +++ b/mojo/edk/system/data_pipe_producer_dispatcher.cc @@ -385,6 +385,7 @@ DataPipeProducerDispatcher::Deserialize(const void* data, scoped_refptr ring_buffer = PlatformSharedBuffer::CreateFromPlatformHandle( state->options.capacity_num_bytes, + false /* read_only */, ScopedPlatformHandle(buffer_handle)); if (!ring_buffer) { DLOG(ERROR) << "Failed to deserialize shared buffer handle."; diff --git a/mojo/edk/system/shared_buffer_dispatcher.cc b/mojo/edk/system/shared_buffer_dispatcher.cc index 4c548e0c0e8149..d531989c1032aa 100644 --- a/mojo/edk/system/shared_buffer_dispatcher.cc +++ b/mojo/edk/system/shared_buffer_dispatcher.cc @@ -26,8 +26,12 @@ namespace { struct SerializedState { uint64_t num_bytes; + uint32_t flags; + uint32_t padding; }; +const uint32_t kSerializedStateFlagsReadOnly = 1 << 0; + #pragma pack(pop) static_assert(sizeof(SerializedState) % 8 == 0, @@ -141,9 +145,10 @@ scoped_refptr SharedBufferDispatcher::Deserialize( // Wrapping |platform_handle| in a |ScopedPlatformHandle| means that it'll be // closed even if creation fails. + bool read_only = (serialization->flags & kSerializedStateFlagsReadOnly); scoped_refptr shared_buffer( PlatformSharedBuffer::CreateFromPlatformHandle( - static_cast(serialization->num_bytes), + static_cast(serialization->num_bytes), read_only, ScopedPlatformHandle(platform_handle))); if (!shared_buffer) { LOG(ERROR) @@ -190,6 +195,21 @@ MojoResult SharedBufferDispatcher::DuplicateBufferHandle( base::AutoLock lock(lock_); if (in_transit_) return MOJO_RESULT_INVALID_ARGUMENT; + + if ((validated_options.flags & + MOJO_DUPLICATE_BUFFER_HANDLE_OPTIONS_FLAG_READ_ONLY) && + (!shared_buffer_->IsReadOnly())) { + // If a read-only duplicate is requested and |shared_buffer_| is not + // read-only, make a read-only duplicate of |shared_buffer_|. + scoped_refptr read_only_buffer = + shared_buffer_->CreateReadOnlyDuplicate(); + if (!read_only_buffer) + return MOJO_RESULT_FAILED_PRECONDITION; + DCHECK(read_only_buffer->IsReadOnly()); + *new_dispatcher = CreateInternal(std::move(read_only_buffer)); + return MOJO_RESULT_OK; + } + *new_dispatcher = CreateInternal(shared_buffer_); return MOJO_RESULT_OK; } @@ -215,8 +235,10 @@ MojoResult SharedBufferDispatcher::MapBuffer( DCHECK(mapping); *mapping = shared_buffer_->MapNoCheck(static_cast(offset), static_cast(num_bytes)); - if (!*mapping) + if (!*mapping) { + LOG(ERROR) << "Unable to map: read_only" << shared_buffer_->IsReadOnly(); return MOJO_RESULT_RESOURCE_EXHAUSTED; + } return MOJO_RESULT_OK; } @@ -237,6 +259,9 @@ bool SharedBufferDispatcher::EndSerialize(void* destination, base::AutoLock lock(lock_); serialization->num_bytes = static_cast(shared_buffer_->GetNumBytes()); + serialization->flags = + (shared_buffer_->IsReadOnly() ? kSerializedStateFlagsReadOnly : 0); + serialization->padding = 0; handle_for_transit_ = shared_buffer_->DuplicatePlatformHandle(); if (!handle_for_transit_.is_valid()) { @@ -283,7 +308,7 @@ MojoResult SharedBufferDispatcher::ValidateDuplicateOptions( const MojoDuplicateBufferHandleOptions* in_options, MojoDuplicateBufferHandleOptions* out_options) { const MojoDuplicateBufferHandleOptionsFlags kKnownFlags = - MOJO_DUPLICATE_BUFFER_HANDLE_OPTIONS_FLAG_NONE; + MOJO_DUPLICATE_BUFFER_HANDLE_OPTIONS_FLAG_READ_ONLY; static const MojoDuplicateBufferHandleOptions kDefaultOptions = { static_cast(sizeof(MojoDuplicateBufferHandleOptions)), MOJO_DUPLICATE_BUFFER_HANDLE_OPTIONS_FLAG_NONE}; diff --git a/mojo/edk/system/shared_buffer_dispatcher_unittest.cc b/mojo/edk/system/shared_buffer_dispatcher_unittest.cc index 5605264172a350..6e26bf9f1bd897 100644 --- a/mojo/edk/system/shared_buffer_dispatcher_unittest.cc +++ b/mojo/edk/system/shared_buffer_dispatcher_unittest.cc @@ -220,6 +220,8 @@ TEST_F(SharedBufferDispatcherTest, DuplicateBufferHandleOptionsValid) { MojoDuplicateBufferHandleOptions options[] = { {sizeof(MojoDuplicateBufferHandleOptions), MOJO_DUPLICATE_BUFFER_HANDLE_OPTIONS_FLAG_NONE}, + {sizeof(MojoDuplicateBufferHandleOptions), + MOJO_DUPLICATE_BUFFER_HANDLE_OPTIONS_FLAG_READ_ONLY}, {sizeof(MojoDuplicateBufferHandleOptionsFlags), ~0u}}; for (size_t i = 0; i < arraysize(options); i++) { scoped_refptr dispatcher2; @@ -227,6 +229,10 @@ TEST_F(SharedBufferDispatcherTest, DuplicateBufferHandleOptionsValid) { &options[i], &dispatcher2)); ASSERT_TRUE(dispatcher2); EXPECT_EQ(Dispatcher::Type::SHARED_BUFFER, dispatcher2->GetType()); + { + scoped_ptr mapping; + EXPECT_EQ(MOJO_RESULT_OK, dispatcher2->MapBuffer(0, 100, 0, &mapping)); + } EXPECT_EQ(MOJO_RESULT_OK, dispatcher2->Close()); } diff --git a/mojo/edk/system/shared_buffer_unittest.cc b/mojo/edk/system/shared_buffer_unittest.cc index 210c2f55f0f16d..9451ae865b3b6d 100644 --- a/mojo/edk/system/shared_buffer_unittest.cc +++ b/mojo/edk/system/shared_buffer_unittest.cc @@ -8,6 +8,7 @@ #include #include "base/logging.h" +#include "base/memory/shared_memory.h" #include "base/strings/string_piece.h" #include "mojo/edk/test/mojo_test_base.h" #include "mojo/public/c/system/types.h" @@ -31,7 +32,7 @@ TEST_F(SharedBufferTest, DuplicateSharedBuffer) { MojoHandle h = CreateBuffer(message.size()); WriteToBuffer(h, 0, message); - MojoHandle dupe = DuplicateBuffer(h); + MojoHandle dupe = DuplicateBuffer(h, false); ExpectBufferContents(dupe, 0, message); } @@ -40,7 +41,7 @@ TEST_F(SharedBufferTest, PassSharedBufferLocal) { MojoHandle h = CreateBuffer(message.size()); WriteToBuffer(h, 0, message); - MojoHandle dupe = DuplicateBuffer(h); + MojoHandle dupe = DuplicateBuffer(h, false); MojoHandle p0, p1; CreateMessagePipe(&p0, &p1); @@ -73,7 +74,7 @@ TEST_F(SharedBufferTest, MAYBE_PassSharedBufferCrossProcess) { MojoHandle b = CreateBuffer(message.size()); RUN_CHILD_ON_PIPE(CopyToBufferClient, h) - MojoHandle dupe = DuplicateBuffer(b); + MojoHandle dupe = DuplicateBuffer(b, false); WriteMessageWithHandles(h, message, &dupe, 1); WriteMessage(h, "quit"); END_CHILD() @@ -120,7 +121,7 @@ DEFINE_TEST_CLIENT_TEST_WITH_PIPE(CreateAndPassBuffer, SharedBufferTest, h) { MojoHandle b = CreateBuffer(message.size()); // Send a copy of the buffer to the parent and the other child. - MojoHandle dupe = DuplicateBuffer(b); + MojoHandle dupe = DuplicateBuffer(b, false); WriteMessageWithHandles(h, "", &b, 1); WriteMessageWithHandles(other_child, "", &dupe, 1); @@ -254,6 +255,93 @@ TEST_F(SharedBufferTest, MAYBE_PassHandleBetweenCousins) { ExpectBufferContents(b, 0, message); } +DEFINE_TEST_CLIENT_TEST_WITH_PIPE(ReadAndMapWriteSharedBuffer, + SharedBufferTest, h) { + // Receive the shared buffer. + MojoHandle b; + EXPECT_EQ("hello", ReadMessageWithHandles(h, &b, 1)); + + // Read from the bufer. + ExpectBufferContents(b, 0, "hello"); + + // Extract the shared memory handle and try to map it writable. + base::SharedMemoryHandle shm_handle; + bool read_only = false; + ASSERT_EQ(MOJO_RESULT_OK, + PassSharedMemoryHandle(b, &shm_handle, nullptr, &read_only)); + base::SharedMemory shared_memory(shm_handle, false); + EXPECT_TRUE(read_only); + EXPECT_FALSE(shared_memory.Map(1234)); + + EXPECT_EQ("quit", ReadMessage(h)); + WriteMessage(h, "ok"); +} + +#if defined(OS_ANDROID) +// Android multi-process tests are not executing the new process. This is flaky. +#define MAYBE_CreateAndPassReadOnlyBuffer DISABLED_CreateAndPassReadOnlyBuffer +#else +#define MAYBE_CreateAndPassReadOnlyBuffer CreateAndPassReadOnlyBuffer +#endif +TEST_F(SharedBufferTest, MAYBE_CreateAndPassReadOnlyBuffer) { + RUN_CHILD_ON_PIPE(ReadAndMapWriteSharedBuffer, h) + // Create a new shared buffer. + MojoHandle b = CreateBuffer(1234); + WriteToBuffer(b, 0, "hello"); + + // Send a read-only copy of the buffer to the child. + MojoHandle dupe = DuplicateBuffer(b, true /* read_only */); + WriteMessageWithHandles(h, "hello", &dupe, 1); + + WriteMessage(h, "quit"); + EXPECT_EQ("ok", ReadMessage(h)); + END_CHILD() +} + +DEFINE_TEST_CLIENT_TEST_WITH_PIPE(CreateAndPassReadOnlyBuffer, + SharedBufferTest, h) { + // Create a new shared buffer. + MojoHandle b = CreateBuffer(1234); + WriteToBuffer(b, 0, "hello"); + + // Send a read-only copy of the buffer to the parent. + MojoHandle dupe = DuplicateBuffer(b, true /* read_only */); + WriteMessageWithHandles(h, "", &dupe, 1); + + EXPECT_EQ("quit", ReadMessage(h)); + WriteMessage(h, "ok"); +} + +#if defined(OS_ANDROID) || (defined(OS_POSIX) && !defined(OS_MACOSX)) +// Android multi-process tests are not executing the new process. This is flaky. +// Non-OSX posix uses a sync broker to create shared memory. Creating read-only +// duplicates in child processes is not currently supported via the sync broker. +#define MAYBE_CreateAndPassFromChildReadOnlyBuffer \ + DISABLED_CreateAndPassFromChildReadOnlyBuffer +#else +#define MAYBE_CreateAndPassFromChildReadOnlyBuffer \ + CreateAndPassFromChildReadOnlyBuffer +#endif +TEST_F(SharedBufferTest, MAYBE_CreateAndPassFromChildReadOnlyBuffer) { + RUN_CHILD_ON_PIPE(CreateAndPassReadOnlyBuffer, h) + MojoHandle b; + EXPECT_EQ("", ReadMessageWithHandles(h, &b, 1)); + ExpectBufferContents(b, 0, "hello"); + + // Extract the shared memory handle and try to map it writable. + base::SharedMemoryHandle shm_handle; + bool read_only = false; + ASSERT_EQ(MOJO_RESULT_OK, + PassSharedMemoryHandle(b, &shm_handle, nullptr, &read_only)); + base::SharedMemory shared_memory(shm_handle, false); + EXPECT_TRUE(read_only); + EXPECT_FALSE(shared_memory.Map(1234)); + + WriteMessage(h, "quit"); + EXPECT_EQ("ok", ReadMessage(h)); + END_CHILD() +} + #endif // !defined(OS_IOS) } // namespace diff --git a/mojo/edk/test/mojo_test_base.cc b/mojo/edk/test/mojo_test_base.cc index 4b044f6334c9f0..b9b68604ca1551 100644 --- a/mojo/edk/test/mojo_test_base.cc +++ b/mojo/edk/test/mojo_test_base.cc @@ -224,10 +224,16 @@ MojoHandle MojoTestBase::CreateBuffer(uint64_t size) { } // static -MojoHandle MojoTestBase::DuplicateBuffer(MojoHandle h) { +MojoHandle MojoTestBase::DuplicateBuffer(MojoHandle h, bool read_only) { MojoHandle new_handle; + MojoDuplicateBufferHandleOptions options = { + sizeof(MojoDuplicateBufferHandleOptions), + MOJO_DUPLICATE_BUFFER_HANDLE_OPTIONS_FLAG_NONE + }; + if (read_only) + options.flags |= MOJO_DUPLICATE_BUFFER_HANDLE_OPTIONS_FLAG_READ_ONLY; EXPECT_EQ(MOJO_RESULT_OK, - MojoDuplicateBufferHandle(h, nullptr, &new_handle)); + MojoDuplicateBufferHandle(h, &options, &new_handle)); return new_handle; } diff --git a/mojo/edk/test/mojo_test_base.h b/mojo/edk/test/mojo_test_base.h index 1dcadf1898b018..0bae23878405a2 100644 --- a/mojo/edk/test/mojo_test_base.h +++ b/mojo/edk/test/mojo_test_base.h @@ -115,7 +115,7 @@ class MojoTestBase : public testing::Test { static MojoHandle CreateBuffer(uint64_t size); // Duplicates a shared buffer to a new handle. - static MojoHandle DuplicateBuffer(MojoHandle h); + static MojoHandle DuplicateBuffer(MojoHandle h, bool read_only); // Maps a buffer, writes some data into it, and unmaps it. static void WriteToBuffer(MojoHandle h, diff --git a/mojo/public/c/system/buffer.h b/mojo/public/c/system/buffer.h index 97f4de4726f9ff..b6d905b7fccfd9 100644 --- a/mojo/public/c/system/buffer.h +++ b/mojo/public/c/system/buffer.h @@ -56,6 +56,9 @@ MOJO_STATIC_ASSERT(sizeof(MojoCreateSharedBufferOptions) == 8, // |MojoDuplicateBufferHandleOptionsFlags flags|: Reserved for future use. // |MOJO_DUPLICATE_BUFFER_HANDLE_OPTIONS_FLAG_NONE|: No flags; default // mode. +// |MOJO_DUPLICATE_BUFFER_HANDLE_OPTIONS_FLAG_READ_ONLY|: The duplicate +// shared buffer can only be mapped read-only. A read-only duplicate can +// only be created before the buffer is passed over a message pipe. // // TODO(vtl): Add flags to remove writability (and executability)? Also, COW? @@ -64,6 +67,8 @@ typedef uint32_t MojoDuplicateBufferHandleOptionsFlags; #ifdef __cplusplus const MojoDuplicateBufferHandleOptionsFlags MOJO_DUPLICATE_BUFFER_HANDLE_OPTIONS_FLAG_NONE = 0; +const MojoDuplicateBufferHandleOptionsFlags + MOJO_DUPLICATE_BUFFER_HANDLE_OPTIONS_FLAG_READ_ONLY = 1 << 0; #else #define MOJO_DUPLICATE_BUFFER_HANDLE_OPTIONS_FLAG_NONE \ ((MojoDuplicateBufferHandleOptionsFlags)0)