Bug 1732343 - Part 5: Use attached handles for Windows SharedMemory serialization, r=handyman

Differential Revision: https://phabricator.services.mozilla.com/D126567
This commit is contained in:
Nika Layzell 2021-11-04 19:20:19 +00:00
parent aa3b31b0e4
commit fc6ef2a44a
11 changed files with 43 additions and 106 deletions

View File

@ -252,8 +252,10 @@ class FontList {
if (aIndex >= mReadOnlyShmems.Length()) {
// Block index out of range
*aOut = base::SharedMemory::NULLHandle();
return;
}
if (!mReadOnlyShmems[aIndex]->ShareToProcess(aPid, aOut)) {
*aOut = mReadOnlyShmems[aIndex]->CloneHandle();
if (!*aOut) {
MOZ_CRASH("failed to share block");
}
}

View File

@ -828,9 +828,8 @@ void FontList::ShareBlocksToProcess(nsTArray<base::SharedMemoryHandle>* aBlocks,
base::ProcessId aPid) {
MOZ_RELEASE_ASSERT(mReadOnlyShmems.Length() == mBlocks.Length());
for (auto& shmem : mReadOnlyShmems) {
base::SharedMemoryHandle* handle =
aBlocks->AppendElement(base::SharedMemory::NULLHandle());
if (!shmem->ShareToProcess(aPid, handle)) {
auto handle = shmem->CloneHandle();
if (!handle) {
// If something went wrong here, we just bail out; the child will need to
// request the blocks as needed, at some performance cost. (Although in
// practice this may mean resources are so constrained the child process
@ -838,6 +837,7 @@ void FontList::ShareBlocksToProcess(nsTArray<base::SharedMemoryHandle>* aBlocks,
aBlocks->Clear();
return;
}
aBlocks->AppendElement(std::move(handle));
}
}
@ -847,12 +847,7 @@ base::SharedMemoryHandle FontList::ShareBlockToProcess(uint32_t aIndex,
MOZ_RELEASE_ASSERT(mReadOnlyShmems.Length() == mBlocks.Length());
MOZ_RELEASE_ASSERT(aIndex < mReadOnlyShmems.Length());
base::SharedMemoryHandle handle = base::SharedMemory::NULLHandle();
if (mReadOnlyShmems[aIndex]->ShareToProcess(aPid, &handle)) {
return handle;
}
return base::SharedMemory::NULLHandle();
return mReadOnlyShmems[aIndex]->CloneHandle();
}
Pointer FontList::Alloc(uint32_t aSize) {

View File

@ -496,9 +496,6 @@ void nsHyphenator::ShareToProcess(base::ProcessId aPid,
if (!mDict.is<UniquePtr<base::SharedMemory>>()) {
return;
}
if (!mDict.as<UniquePtr<base::SharedMemory>>()->ShareToProcess(aPid,
aOutHandle)) {
return;
}
*aOutHandle = mDict.as<UniquePtr<base::SharedMemory>>()->CloneHandle();
*aOutSize = mDictSize;
}

View File

@ -24,11 +24,7 @@ namespace base {
// SharedMemoryHandle is a platform specific type which represents
// the underlying OS handle to a shared memory segment.
#if defined(OS_WIN)
typedef HANDLE SharedMemoryHandle;
#elif defined(OS_POSIX)
typedef mozilla::UniqueFileHandle SharedMemoryHandle;
#endif
// Platform abstraction for shared memory. Provides a C++ wrapper
// around the OS primitive for a memory mapped file.
@ -97,16 +93,20 @@ class SharedMemory {
// Mapped via Map(). Returns NULL if it is not mapped.
void* memory() const { return memory_.get(); }
// Extracts the underlying file handle; similar to
// GiveToProcess(GetCurrentProcId(), ...) but returns a RAII type.
// Like GiveToProcess, this unmaps the memory as a side-effect (and
// cleans up any OS-specific resources).
// Extracts the underlying file handle, returning a RAII type.
// This unmaps the memory as a side-effect (and cleans up any OS-specific
// resources).
mozilla::UniqueFileHandle TakeHandle() {
mozilla::UniqueFileHandle handle = std::move(mapped_file_);
Close();
return handle;
}
// Creates a copy of the underlying file handle, returning a RAII type.
// This operation may fail, in which case the returned file handle will be
// invalid.
mozilla::UniqueFileHandle CloneHandle();
// Make the shared memory object read-only, such that it cannot be
// written even if it's sent to an untrusted process. If it was
// mapped in this process, it will be unmapped. The object must
@ -149,27 +149,6 @@ class SharedMemory {
// something there in the meantime.
static void* FindFreeAddressSpace(size_t size);
// Share the shared memory to another process. Attempts
// to create a platform-specific new_handle which can be
// used in a remote process to access the shared memory
// file. new_handle is an ouput parameter to receive
// the handle for use in the remote process.
// Returns true on success, false otherwise.
bool ShareToProcess(base::ProcessId target_pid,
SharedMemoryHandle* new_handle) {
return ShareToProcessCommon(target_pid, new_handle, false);
}
// Logically equivalent to:
// bool ok = ShareToProcess(process, new_handle);
// Close();
// return ok;
// Note that the memory is unmapped by calling this method, regardless of the
// return value.
bool GiveToProcess(ProcessId target_pid, SharedMemoryHandle* new_handle) {
return ShareToProcessCommon(target_pid, new_handle, true);
}
#ifdef OS_POSIX
// If named POSIX shm is being used, append the prefix (including
// the leading '/') that would be used by a process with the given
@ -179,9 +158,6 @@ class SharedMemory {
#endif
private:
bool ShareToProcessCommon(ProcessId target_pid,
SharedMemoryHandle* new_handle, bool close_self);
bool CreateInternal(size_t size, bool freezeable);
// Unmapping shared memory requires the mapped size on Unix but not

View File

@ -527,23 +527,16 @@ void* SharedMemory::FindFreeAddressSpace(size_t size) {
return memory != MAP_FAILED ? memory : NULL;
}
bool SharedMemory::ShareToProcessCommon(ProcessId processId,
SharedMemoryHandle* new_handle,
bool close_self) {
SharedMemoryHandle SharedMemory::CloneHandle() {
freezeable_ = false;
const int new_fd = dup(mapped_file_.get());
if (new_fd < 0) {
CHROMIUM_LOG(WARNING) << "failed to duplicate file descriptor: "
<< strerror(errno);
return false;
return nullptr;
}
*new_handle = mozilla::UniqueFileHandle(new_fd);
if (close_self) Close();
return true;
return mozilla::UniqueFileHandle(new_fd);
}
void SharedMemory::Close(bool unmap_view) {
if (unmap_view) {
Unmap();

View File

@ -69,7 +69,7 @@ bool SharedMemory::SetHandle(SharedMemoryHandle handle, bool read_only) {
external_section_ = true;
freezeable_ = false; // just in case
mapped_file_.reset(handle);
mapped_file_ = std::move(handle);
read_only_ = read_only;
return true;
}
@ -191,39 +191,16 @@ void* SharedMemory::FindFreeAddressSpace(size_t size) {
return memory;
}
bool SharedMemory::ShareToProcessCommon(ProcessId processId,
SharedMemoryHandle* new_handle,
bool close_self) {
SharedMemoryHandle SharedMemory::CloneHandle() {
freezeable_ = false;
*new_handle = 0;
DWORD access = FILE_MAP_READ | SECTION_QUERY;
DWORD options = 0;
HANDLE mapped_file;
HANDLE result;
if (!read_only_) {
access |= FILE_MAP_WRITE;
HANDLE handle = INVALID_HANDLE_VALUE;
if (DuplicateHandle(GetCurrentProcess(), mapped_file_.get(),
GetCurrentProcess(), &handle, 0, false,
DUPLICATE_SAME_ACCESS)) {
return SharedMemoryHandle(handle);
}
if (close_self) {
// DUPLICATE_CLOSE_SOURCE causes DuplicateHandle to close mapped_file.
mapped_file = mapped_file_.release();
options = DUPLICATE_CLOSE_SOURCE;
Unmap();
} else {
mapped_file = mapped_file_.get();
}
if (processId == GetCurrentProcId() && close_self) {
*new_handle = mapped_file;
return true;
}
if (!mozilla::ipc::DuplicateHandle(mapped_file, processId, &result, access,
options)) {
return false;
}
*new_handle = result;
return true;
NS_WARNING("DuplicateHandle Failed!");
return nullptr;
}
void SharedMemory::Close(bool unmap_view) {

View File

@ -191,9 +191,8 @@ IPCResult IdleSchedulerParent::RecvInitForIdleUse(
}
}
Maybe<SharedMemoryHandle> activeCounter;
SharedMemoryHandle handle;
if (sActiveChildCounter &&
sActiveChildCounter->ShareToProcess(OtherPid(), &handle)) {
if (SharedMemoryHandle handle =
sActiveChildCounter ? sActiveChildCounter->CloneHandle() : nullptr) {
activeCounter.emplace(std::move(handle));
}

View File

@ -112,7 +112,7 @@ bool SharedPreferenceDeserializer::DeserializeFromSharedMemory(
Maybe<base::SharedMemoryHandle> prefsHandle;
#ifdef XP_WIN
prefsHandle = Some(HANDLE((uintptr_t)(aPrefsHandle)));
prefsHandle = Some(UniqueFileHandle(HANDLE((uintptr_t)(aPrefsHandle))));
if (!aPrefsHandle) {
return false;
}

View File

@ -73,10 +73,8 @@ class SharedMemoryBasic final
virtual bool ShareToProcess(base::ProcessId aProcessId,
Handle* new_handle) override {
base::SharedMemoryHandle handle;
bool ret = mSharedMemory.ShareToProcess(aProcessId, &handle);
if (ret) *new_handle = std::move(handle);
return ret;
*new_handle = mSharedMemory.CloneHandle();
return base::SharedMemory::IsHandleValid(*new_handle);
}
static void* FindFreeAddressSpace(size_t size) {

View File

@ -46,8 +46,7 @@ TEST(IPCSharedMemory, FreezeAndMapRW)
ASSERT_FALSE(shm.memory());
// Re-create as writeable
auto handle = base::SharedMemory::NULLHandle();
ASSERT_TRUE(shm.GiveToProcess(base::GetCurrentProcId(), &handle));
auto handle = shm.TakeHandle();
ASSERT_TRUE(shm.IsHandleValid(handle));
ASSERT_FALSE(shm.IsValid());
ASSERT_TRUE(shm.SetHandle(std::move(handle), /* read-only */ false));
@ -103,8 +102,7 @@ TEST(IPCSharedMemory, Reprotect)
*mem = 'A';
// Re-create as read-only
auto handle = base::SharedMemory::NULLHandle();
ASSERT_TRUE(shm.GiveToProcess(base::GetCurrentProcId(), &handle));
auto handle = shm.TakeHandle();
ASSERT_TRUE(shm.IsHandleValid(handle));
ASSERT_FALSE(shm.IsValid());
ASSERT_TRUE(shm.SetHandle(std::move(handle), /* read-only */ true));
@ -141,14 +139,14 @@ TEST(IPCSharedMemory, WinUnfreeze)
ASSERT_FALSE(shm.memory());
// Extract handle.
auto handle = base::SharedMemory::NULLHandle();
ASSERT_TRUE(shm.GiveToProcess(base::GetCurrentProcId(), &handle));
auto handle = shm.TakeHandle();
ASSERT_TRUE(shm.IsHandleValid(handle));
ASSERT_FALSE(shm.IsValid());
// Unfreeze.
HANDLE newHandle = INVALID_HANDLE_VALUE;
bool unfroze = ::DuplicateHandle(
GetCurrentProcess(), handle, GetCurrentProcess(), &handle,
GetCurrentProcess(), handle.release(), GetCurrentProcess(), &newHandle,
FILE_MAP_ALL_ACCESS, false, DUPLICATE_CLOSE_SOURCE);
ASSERT_FALSE(unfroze);
}
@ -235,8 +233,7 @@ TEST(IPCSharedMemory, ROCopyAndMapRW)
ASSERT_TRUE(shmRO.IsValid());
// Re-create as writeable
auto handle = base::SharedMemory::NULLHandle();
ASSERT_TRUE(shmRO.GiveToProcess(base::GetCurrentProcId(), &handle));
auto handle = shmRO.TakeHandle();
ASSERT_TRUE(shmRO.IsHandleValid(handle));
ASSERT_FALSE(shmRO.IsValid());
ASSERT_TRUE(shmRO.SetHandle(std::move(handle), /* read-only */ false));

View File

@ -688,7 +688,10 @@ bool GlobalStyleSheetCache::AffectedByPref(const nsACString& aPref) {
bool GlobalStyleSheetCache::ShareToProcess(base::ProcessId aProcessId,
base::SharedMemoryHandle* aHandle) {
MOZ_ASSERT(XRE_IsParentProcess());
return sSharedMemory && sSharedMemory->ShareToProcess(aProcessId, aHandle);
if (sSharedMemory) {
*aHandle = sSharedMemory->CloneHandle();
}
return !!*aHandle;
}
StaticRefPtr<GlobalStyleSheetCache> GlobalStyleSheetCache::gStyleCache;