Replace ReadOnlySharedMemoryRegion with WritableSharedMemoryRegion (see #3502)

Write access to the shared memory region is required because JavaScript lacks
the capability to create read-only ArrayBuffers. When a user attempts to modify
an ArrayBuffer that utilizes a ReadOnlySharedMemoryRegion as its BackingStore
it triggers an access violation.

Note that this pull request may be reverted in the future if JavaScript adds
read-only ArrayBuffer support.
This commit is contained in:
Nik Pavlov 2023-10-16 22:43:29 +00:00 committed by Marshall Greenblatt
parent f3e92b45fc
commit 8d7001adc3
16 changed files with 81 additions and 42 deletions

View File

@ -33,7 +33,7 @@
// by hand. See the translator.README.txt file in the tools directory for // by hand. See the translator.README.txt file in the tools directory for
// more information. // more information.
// //
// $hash=08f64795d78bdad29a45222a7263e795ce77a52d$ // $hash=dfa2f2d57339e05592d7ee5f4c4c54dd0932cd94$
// //
#ifndef CEF_INCLUDE_CAPI_CEF_SHARED_MEMORY_REGION_CAPI_H_ #ifndef CEF_INCLUDE_CAPI_CEF_SHARED_MEMORY_REGION_CAPI_H_
@ -69,7 +69,7 @@ typedef struct _cef_shared_memory_region_t {
/// Returns the pointer to the memory. Returns nullptr for invalid instances. /// Returns the pointer to the memory. Returns nullptr for invalid instances.
/// The returned pointer is only valid for the life span of this object. /// The returned pointer is only valid for the life span of this object.
/// ///
const void*(CEF_CALLBACK* memory)(struct _cef_shared_memory_region_t* self); void*(CEF_CALLBACK* memory)(struct _cef_shared_memory_region_t* self);
} cef_shared_memory_region_t; } cef_shared_memory_region_t;
#ifdef __cplusplus #ifdef __cplusplus

View File

@ -42,13 +42,13 @@
// way that may cause binary incompatibility with other builds. The universal // way that may cause binary incompatibility with other builds. The universal
// hash value will change if any platform is affected whereas the platform hash // hash value will change if any platform is affected whereas the platform hash
// values will change only if that particular platform is affected. // values will change only if that particular platform is affected.
#define CEF_API_HASH_UNIVERSAL "f8f0a0431b4cf46f98e3e58af93199f21d403d69" #define CEF_API_HASH_UNIVERSAL "e5e3eef669443880f747ebccf04ca470306acec9"
#if defined(OS_WIN) #if defined(OS_WIN)
#define CEF_API_HASH_PLATFORM "e5142124a2c423c0c438909bbfb86cd35a2b839d" #define CEF_API_HASH_PLATFORM "2a84aaf4b26c613183b48b4aac39aa57045287af"
#elif defined(OS_MAC) #elif defined(OS_MAC)
#define CEF_API_HASH_PLATFORM "5748f4bc71b501c804ef284e091e6eea847086d9" #define CEF_API_HASH_PLATFORM "6e409fac3d2b069c084bc17a04196d4a93d5c028"
#elif defined(OS_LINUX) #elif defined(OS_LINUX)
#define CEF_API_HASH_PLATFORM "23413591633deaf08aae5248becdb0118b105de1" #define CEF_API_HASH_PLATFORM "18d2e48f53384ccb092b7dd4f0ae5dceafdac1fa"
#endif #endif
#ifdef __cplusplus #ifdef __cplusplus

View File

@ -63,7 +63,7 @@ class CefSharedMemoryRegion : public virtual CefBaseRefCounted {
/// The returned pointer is only valid for the life span of this object. /// The returned pointer is only valid for the life span of this object.
/// ///
/*--cef()--*/ /*--cef()--*/
virtual const void* Memory() = 0; virtual void* Memory() = 0;
}; };
#endif // CEF_INCLUDE_CEF_SHARED_MEMORY_REGION_H_ #endif // CEF_INCLUDE_CEF_SHARED_MEMORY_REGION_H_

View File

@ -44,7 +44,7 @@ void CefBrowserFrame::SendMessage(const std::string& name,
void CefBrowserFrame::SendSharedMemoryRegion( void CefBrowserFrame::SendSharedMemoryRegion(
const std::string& name, const std::string& name,
base::ReadOnlySharedMemoryRegion region) { base::WritableSharedMemoryRegion region) {
// Always send to the newly created RFH, which may be speculative when // Always send to the newly created RFH, which may be speculative when
// navigating cross-origin. // navigating cross-origin.
if (auto host = GetFrameHost(/*prefer_speculative=*/true)) { if (auto host = GetFrameHost(/*prefer_speculative=*/true)) {

View File

@ -38,7 +38,7 @@ class CefBrowserFrame
void SendMessage(const std::string& name, void SendMessage(const std::string& name,
base::Value::List arguments) override; base::Value::List arguments) override;
void SendSharedMemoryRegion(const std::string& name, void SendSharedMemoryRegion(const std::string& name,
base::ReadOnlySharedMemoryRegion region) override; base::WritableSharedMemoryRegion region) override;
void FrameAttached(mojo::PendingRemote<cef::mojom::RenderFrame> render_frame, void FrameAttached(mojo::PendingRemote<cef::mojom::RenderFrame> render_frame,
bool reattached) override; bool reattached) override;
void UpdateDraggableRegions( void UpdateDraggableRegions(

View File

@ -289,7 +289,7 @@ void CefFrameHostImpl::SendProcessMessage(
SendToRenderFrame( SendToRenderFrame(
__FUNCTION__, __FUNCTION__,
base::BindOnce( base::BindOnce(
[](const CefString& name, base::ReadOnlySharedMemoryRegion region, [](const CefString& name, base::WritableSharedMemoryRegion region,
const RenderFrameType& render_frame) { const RenderFrameType& render_frame) {
render_frame->SendSharedMemoryRegion(name, std::move(region)); render_frame->SendSharedMemoryRegion(name, std::move(region));
}, },
@ -644,7 +644,7 @@ void CefFrameHostImpl::SendMessage(const std::string& name,
void CefFrameHostImpl::SendSharedMemoryRegion( void CefFrameHostImpl::SendSharedMemoryRegion(
const std::string& name, const std::string& name,
base::ReadOnlySharedMemoryRegion region) { base::WritableSharedMemoryRegion region) {
if (auto browser = GetBrowserHostBase()) { if (auto browser = GetBrowserHostBase()) {
if (auto client = browser->GetClient()) { if (auto client = browser->GetClient()) {
CefRefPtr<CefProcessMessage> message( CefRefPtr<CefProcessMessage> message(

View File

@ -138,7 +138,7 @@ class CefFrameHostImpl : public CefFrame, public cef::mojom::BrowserFrame {
void SendMessage(const std::string& name, void SendMessage(const std::string& name,
base::Value::List arguments) override; base::Value::List arguments) override;
void SendSharedMemoryRegion(const std::string& name, void SendSharedMemoryRegion(const std::string& name,
base::ReadOnlySharedMemoryRegion region) override; base::WritableSharedMemoryRegion region) override;
void FrameAttached(mojo::PendingRemote<cef::mojom::RenderFrame> render_frame, void FrameAttached(mojo::PendingRemote<cef::mojom::RenderFrame> render_frame,
bool reattached) override; bool reattached) override;
void UpdateDraggableRegions( void UpdateDraggableRegions(

View File

@ -59,7 +59,7 @@ interface RenderFrame {
SendMessage(string name, mojo_base.mojom.ListValue arguments); SendMessage(string name, mojo_base.mojom.ListValue arguments);
// Send a shared memory region to the render process. // Send a shared memory region to the render process.
SendSharedMemoryRegion(string name, mojo_base.mojom.ReadOnlySharedMemoryRegion region); SendSharedMemoryRegion(string name, mojo_base.mojom.WritableSharedMemoryRegion region);
// Send a command. // Send a command.
SendCommand(string command); SendCommand(string command);
@ -91,7 +91,7 @@ interface BrowserFrame {
SendMessage(string name, mojo_base.mojom.ListValue arguments); SendMessage(string name, mojo_base.mojom.ListValue arguments);
// Send a shared memory region to the browser process. // Send a shared memory region to the browser process.
SendSharedMemoryRegion(string name, mojo_base.mojom.ReadOnlySharedMemoryRegion region); SendSharedMemoryRegion(string name, mojo_base.mojom.WritableSharedMemoryRegion region);
// The render frame is ready to begin handling actions. // The render frame is ready to begin handling actions.
FrameAttached(pending_remote<RenderFrame> render_frame, FrameAttached(pending_remote<RenderFrame> render_frame,

View File

@ -12,7 +12,7 @@ namespace {
class CefSharedMemoryRegionImpl final : public CefSharedMemoryRegion { class CefSharedMemoryRegionImpl final : public CefSharedMemoryRegion {
public: public:
CefSharedMemoryRegionImpl(base::ReadOnlySharedMemoryMapping&& mapping) CefSharedMemoryRegionImpl(base::WritableSharedMemoryMapping&& mapping)
: mapping_(std::move(mapping)) {} : mapping_(std::move(mapping)) {}
CefSharedMemoryRegionImpl(const CefSharedMemoryRegionImpl&) = delete; CefSharedMemoryRegionImpl(const CefSharedMemoryRegionImpl&) = delete;
CefSharedMemoryRegionImpl& operator=(const CefSharedMemoryRegionImpl&) = CefSharedMemoryRegionImpl& operator=(const CefSharedMemoryRegionImpl&) =
@ -21,10 +21,10 @@ class CefSharedMemoryRegionImpl final : public CefSharedMemoryRegion {
// CefSharedMemoryRegion methods // CefSharedMemoryRegion methods
bool IsValid() override { return mapping_.IsValid(); } bool IsValid() override { return mapping_.IsValid(); }
size_t Size() override { return IsValid() ? mapping_.size() : 0; } size_t Size() override { return IsValid() ? mapping_.size() : 0; }
const void* Memory() override { return mapping_.memory(); } void* Memory() override { return mapping_.memory(); }
private: private:
base::ReadOnlySharedMemoryMapping mapping_; base::WritableSharedMemoryMapping mapping_;
IMPLEMENT_REFCOUNTING(CefSharedMemoryRegionImpl); IMPLEMENT_REFCOUNTING(CefSharedMemoryRegionImpl);
}; };
@ -32,7 +32,7 @@ class CefSharedMemoryRegionImpl final : public CefSharedMemoryRegion {
CefProcessMessageSMRImpl::CefProcessMessageSMRImpl( CefProcessMessageSMRImpl::CefProcessMessageSMRImpl(
const CefString& name, const CefString& name,
base::ReadOnlySharedMemoryRegion&& region) base::WritableSharedMemoryRegion&& region)
: name_(name), region_(std::move(region)) { : name_(name), region_(std::move(region)) {
DCHECK(!name_.empty()); DCHECK(!name_.empty());
DCHECK(region_.IsValid()); DCHECK(region_.IsValid());
@ -53,7 +53,7 @@ CefProcessMessageSMRImpl::GetSharedMemoryRegion() {
return new CefSharedMemoryRegionImpl(region_.Map()); return new CefSharedMemoryRegionImpl(region_.Map());
} }
base::ReadOnlySharedMemoryRegion CefProcessMessageSMRImpl::TakeRegion() { base::WritableSharedMemoryRegion CefProcessMessageSMRImpl::TakeRegion() {
return std::move(region_); return std::move(region_);
} }
@ -68,23 +68,27 @@ CefSharedProcessMessageBuilderImpl::CefSharedProcessMessageBuilderImpl(
const CefString& name, const CefString& name,
size_t byte_size) size_t byte_size)
: name_(name), : name_(name),
region_(base::ReadOnlySharedMemoryRegion::Create(byte_size)) {} region_(base::WritableSharedMemoryRegion::Create(byte_size)),
mapping_(region_.Map()) {}
bool CefSharedProcessMessageBuilderImpl::IsValid() { bool CefSharedProcessMessageBuilderImpl::IsValid() {
return region_.region.IsValid() && region_.mapping.IsValid(); return region_.IsValid() && mapping_.IsValid();
} }
size_t CefSharedProcessMessageBuilderImpl::Size() { size_t CefSharedProcessMessageBuilderImpl::Size() {
return !IsValid() ? 0 : region_.mapping.size(); return !IsValid() ? 0 : region_.GetSize();
} }
void* CefSharedProcessMessageBuilderImpl::Memory() { void* CefSharedProcessMessageBuilderImpl::Memory() {
return !IsValid() ? nullptr : region_.mapping.memory(); return !IsValid() ? nullptr : mapping_.memory();
} }
CefRefPtr<CefProcessMessage> CefSharedProcessMessageBuilderImpl::Build() { CefRefPtr<CefProcessMessage> CefSharedProcessMessageBuilderImpl::Build() {
if (!IsValid()) { if (!IsValid()) {
return nullptr; return nullptr;
} }
return new CefProcessMessageSMRImpl(name_, std::move(region_.region));
// Invalidate mappping
mapping_ = base::WritableSharedMemoryMapping();
return new CefProcessMessageSMRImpl(name_, std::move(region_));
} }

View File

@ -9,12 +9,12 @@
#include "include/cef_process_message.h" #include "include/cef_process_message.h"
#include "include/cef_shared_process_message_builder.h" #include "include/cef_shared_process_message_builder.h"
#include "base/memory/read_only_shared_memory_region.h" #include "base/memory/writable_shared_memory_region.h"
class CefProcessMessageSMRImpl final : public CefProcessMessage { class CefProcessMessageSMRImpl final : public CefProcessMessage {
public: public:
CefProcessMessageSMRImpl(const CefString& name, CefProcessMessageSMRImpl(const CefString& name,
base::ReadOnlySharedMemoryRegion&& region); base::WritableSharedMemoryRegion&& region);
CefProcessMessageSMRImpl(const CefProcessMessageSMRImpl&) = delete; CefProcessMessageSMRImpl(const CefProcessMessageSMRImpl&) = delete;
CefProcessMessageSMRImpl& operator=(const CefProcessMessageSMRImpl&) = delete; CefProcessMessageSMRImpl& operator=(const CefProcessMessageSMRImpl&) = delete;
~CefProcessMessageSMRImpl() override; ~CefProcessMessageSMRImpl() override;
@ -26,11 +26,11 @@ class CefProcessMessageSMRImpl final : public CefProcessMessage {
CefString GetName() override; CefString GetName() override;
CefRefPtr<CefListValue> GetArgumentList() override { return nullptr; } CefRefPtr<CefListValue> GetArgumentList() override { return nullptr; }
CefRefPtr<CefSharedMemoryRegion> GetSharedMemoryRegion() override; CefRefPtr<CefSharedMemoryRegion> GetSharedMemoryRegion() override;
[[nodiscard]] base::ReadOnlySharedMemoryRegion TakeRegion(); [[nodiscard]] base::WritableSharedMemoryRegion TakeRegion();
private: private:
const CefString name_; const CefString name_;
base::ReadOnlySharedMemoryRegion region_; base::WritableSharedMemoryRegion region_;
IMPLEMENT_REFCOUNTING(CefProcessMessageSMRImpl); IMPLEMENT_REFCOUNTING(CefProcessMessageSMRImpl);
}; };
@ -50,7 +50,8 @@ class CefSharedProcessMessageBuilderImpl final
private: private:
const CefString name_; const CefString name_;
base::MappedReadOnlyRegion region_; base::WritableSharedMemoryRegion region_;
base::WritableSharedMemoryMapping mapping_;
IMPLEMENT_REFCOUNTING(CefSharedProcessMessageBuilderImpl); IMPLEMENT_REFCOUNTING(CefSharedProcessMessageBuilderImpl);
}; };

View File

@ -289,7 +289,7 @@ void CefFrameImpl::SendProcessMessage(CefProcessId target_process,
SendToBrowserFrame( SendToBrowserFrame(
__FUNCTION__, __FUNCTION__,
base::BindOnce( base::BindOnce(
[](const CefString& name, base::ReadOnlySharedMemoryRegion region, [](const CefString& name, base::WritableSharedMemoryRegion region,
const BrowserFrameType& render_frame) { const BrowserFrameType& render_frame) {
render_frame->SendSharedMemoryRegion(name, std::move(region)); render_frame->SendSharedMemoryRegion(name, std::move(region));
}, },
@ -698,7 +698,7 @@ void CefFrameImpl::SendMessage(const std::string& name,
void CefFrameImpl::SendSharedMemoryRegion( void CefFrameImpl::SendSharedMemoryRegion(
const std::string& name, const std::string& name,
base::ReadOnlySharedMemoryRegion region) { base::WritableSharedMemoryRegion region) {
if (auto app = CefAppManager::Get()->GetApplication()) { if (auto app = CefAppManager::Get()->GetApplication()) {
if (auto handler = app->GetRenderProcessHandler()) { if (auto handler = app->GetRenderProcessHandler()) {
CefRefPtr<CefProcessMessage> message( CefRefPtr<CefProcessMessage> message(

View File

@ -145,7 +145,7 @@ class CefFrameImpl
void SendMessage(const std::string& name, void SendMessage(const std::string& name,
base::Value::List arguments) override; base::Value::List arguments) override;
void SendSharedMemoryRegion(const std::string& name, void SendSharedMemoryRegion(const std::string& name,
base::ReadOnlySharedMemoryRegion region) override; base::WritableSharedMemoryRegion region) override;
void SendCommand(const std::string& command) override; void SendCommand(const std::string& command) override;
void SendCommandWithResponse( void SendCommandWithResponse(
const std::string& command, const std::string& command,

View File

@ -9,7 +9,7 @@
// implementations. See the translator.README.txt file in the tools directory // implementations. See the translator.README.txt file in the tools directory
// for more information. // for more information.
// //
// $hash=9b9187a75a85ff63f2244471af1e54f40eae5a82$ // $hash=3bc6db85e54dc87c1e592291be01820547e0989f$
// //
#include "libcef_dll/cpptoc/shared_memory_region_cpptoc.h" #include "libcef_dll/cpptoc/shared_memory_region_cpptoc.h"
@ -55,7 +55,7 @@ shared_memory_region_size(struct _cef_shared_memory_region_t* self) {
return _retval; return _retval;
} }
const void* CEF_CALLBACK void* CEF_CALLBACK
shared_memory_region_memory(struct _cef_shared_memory_region_t* self) { shared_memory_region_memory(struct _cef_shared_memory_region_t* self) {
shutdown_checker::AssertNotShutdown(); shutdown_checker::AssertNotShutdown();
@ -65,9 +65,9 @@ shared_memory_region_memory(struct _cef_shared_memory_region_t* self) {
} }
// Execute // Execute
const void* _retval = CefSharedMemoryRegionCppToC::Get(self)->Memory(); void* _retval = CefSharedMemoryRegionCppToC::Get(self)->Memory();
// Return type: simple // Return type: simple_byaddr
return _retval; return _retval;
} }

View File

@ -9,7 +9,7 @@
// implementations. See the translator.README.txt file in the tools directory // implementations. See the translator.README.txt file in the tools directory
// for more information. // for more information.
// //
// $hash=79771feab6c6d60667691c826ca9d6deaa23d068$ // $hash=31516110398f9fe682988645d74ac8789b712181$
// //
#include "libcef_dll/ctocpp/shared_memory_region_ctocpp.h" #include "libcef_dll/ctocpp/shared_memory_region_ctocpp.h"
@ -51,7 +51,7 @@ NO_SANITIZE("cfi-icall") size_t CefSharedMemoryRegionCToCpp::Size() {
return _retval; return _retval;
} }
NO_SANITIZE("cfi-icall") const void* CefSharedMemoryRegionCToCpp::Memory() { NO_SANITIZE("cfi-icall") void* CefSharedMemoryRegionCToCpp::Memory() {
shutdown_checker::AssertNotShutdown(); shutdown_checker::AssertNotShutdown();
cef_shared_memory_region_t* _struct = GetStruct(); cef_shared_memory_region_t* _struct = GetStruct();
@ -60,9 +60,9 @@ NO_SANITIZE("cfi-icall") const void* CefSharedMemoryRegionCToCpp::Memory() {
} }
// Execute // Execute
const void* _retval = _struct->memory(_struct); void* _retval = _struct->memory(_struct);
// Return type: simple // Return type: simple_byaddr
return _retval; return _retval;
} }

View File

@ -9,7 +9,7 @@
// implementations. See the translator.README.txt file in the tools directory // implementations. See the translator.README.txt file in the tools directory
// for more information. // for more information.
// //
// $hash=f5d0285d28412c40b8e04953025294c5f0779ecd$ // $hash=a81ba6b7aca8e1f7e6e6ef41e727ddcffc06f204$
// //
#ifndef CEF_LIBCEF_DLL_CTOCPP_SHARED_MEMORY_REGION_CTOCPP_H_ #ifndef CEF_LIBCEF_DLL_CTOCPP_SHARED_MEMORY_REGION_CTOCPP_H_
@ -37,7 +37,7 @@ class CefSharedMemoryRegionCToCpp
// CefSharedMemoryRegion methods. // CefSharedMemoryRegion methods.
bool IsValid() override; bool IsValid() override;
size_t Size() override; size_t Size() override;
const void* Memory() override; void* Memory() override;
}; };
#endif // CEF_LIBCEF_DLL_CTOCPP_SHARED_MEMORY_REGION_CTOCPP_H_ #endif // CEF_LIBCEF_DLL_CTOCPP_SHARED_MEMORY_REGION_CTOCPP_H_

View File

@ -7,6 +7,7 @@
#include "tests/gtest/include/gtest/gtest.h" #include "tests/gtest/include/gtest/gtest.h"
#include <array> #include <array>
#include <type_traits>
namespace { namespace {
@ -29,6 +30,9 @@ CefRefPtr<CefSharedProcessMessageBuilder> CreateTestBuilder() {
EXPECT_NE(builder, nullptr); EXPECT_NE(builder, nullptr);
EXPECT_TRUE(builder->IsValid()); EXPECT_TRUE(builder->IsValid());
static_assert(
std::is_trivially_copyable_v<TestData>,
"Do not copy non-trivially-copyable object across memory spaces");
auto data = static_cast<TestData*>(builder->Memory()); auto data = static_cast<TestData*>(builder->Memory());
EXPECT_NE(data, nullptr); EXPECT_NE(data, nullptr);
@ -92,3 +96,33 @@ TEST(SharedProcessMessageTest,
EXPECT_EQ(read_data->buffer[i], i); EXPECT_EQ(read_data->buffer[i], i);
} }
} }
TEST(SharedProcessMessageTest, WrittenValuesVisibleInOtherRegion) {
CefRefPtr<CefSharedMemoryRegion> read_region;
CefRefPtr<CefSharedMemoryRegion> write_region;
{
auto builder = CreateTestBuilder();
auto message = builder->Build();
read_region = message->GetSharedMemoryRegion();
write_region = message->GetSharedMemoryRegion();
}
EXPECT_TRUE(write_region->IsValid());
auto write_data = static_cast<TestData*>(write_region->Memory());
write_data->flag = !kTestFlag;
write_data->value = kTestValue * 2;
auto new_double_value = kTestDoubleValue * 3;
write_data->doubleValue = new_double_value;
for (size_t i = 0; i < write_data->buffer.size(); ++i) {
write_data->buffer[i] = i + 1;
}
EXPECT_TRUE(read_region->IsValid());
auto read_data = static_cast<const TestData*>(read_region->Memory());
EXPECT_EQ(read_data->flag, !kTestFlag);
EXPECT_EQ(read_data->value, kTestValue * 2);
EXPECT_EQ(read_data->doubleValue, new_double_value);
for (size_t i = 0; i < read_data->buffer.size(); ++i) {
EXPECT_EQ(read_data->buffer[i], i + 1);
}
}