Commit 61e00b8b authored by Tanmoy Mollik's avatar Tanmoy Mollik Committed by Commit Bot

Convert NaCl crash reporting to the new shared memory API

This CL replaces the deprecated base::SharedMemoryHandle used to hold the crash information
in nacl:: classes with new shared memory api. The plugin side writes to the shared memory
region using base::WritableSharedMemoryRegion and sends a read only version of the region
to the renderer process through the browser. The renderer side reads the crash info using
base::ReadOnlySharedMemoryRegion.

Change-Id: I335040a9884d6dc79f107070c954a3ff250458a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1565974Reviewed-by: default avatarDerek Schuff <dschuff@chromium.org>
Reviewed-by: default avatarAlex Ilin <alexilin@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Commit-Queue: Tanmoy Mollik <triploblastic@google.com>
Cr-Commit-Position: refs/heads/master@{#658493}
parent 95774506
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <cstddef> #include <cstddef>
#include <type_traits> #include <type_traits>
#include "base/containers/buffer_iterator.h"
#include "base/containers/span.h" #include "base/containers/span.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/unguessable_token.h" #include "base/unguessable_token.h"
...@@ -145,6 +146,12 @@ class BASE_EXPORT ReadOnlySharedMemoryMapping : public SharedMemoryMapping { ...@@ -145,6 +146,12 @@ class BASE_EXPORT ReadOnlySharedMemoryMapping : public SharedMemoryMapping {
return span<const T>(static_cast<const T*>(raw_memory_ptr()), count); return span<const T>(static_cast<const T*>(raw_memory_ptr()), count);
} }
// Returns a BufferIterator of const T.
template <typename T>
BufferIterator<const T> GetMemoryAsBufferIterator() const {
return BufferIterator<const T>(GetMemoryAsSpan<T>());
}
private: private:
friend class ReadOnlySharedMemoryRegion; friend class ReadOnlySharedMemoryRegion;
ReadOnlySharedMemoryMapping(void* address, ReadOnlySharedMemoryMapping(void* address,
...@@ -216,6 +223,12 @@ class BASE_EXPORT WritableSharedMemoryMapping : public SharedMemoryMapping { ...@@ -216,6 +223,12 @@ class BASE_EXPORT WritableSharedMemoryMapping : public SharedMemoryMapping {
return span<T>(static_cast<T*>(raw_memory_ptr()), count); return span<T>(static_cast<T*>(raw_memory_ptr()), count);
} }
// Returns a BufferIterator of T.
template <typename T>
BufferIterator<T> GetMemoryAsBufferIterator() {
return BufferIterator<T>(GetMemoryAsSpan<T>());
}
private: private:
friend WritableSharedMemoryMapping MapAtForTesting( friend WritableSharedMemoryMapping MapAtForTesting(
subtle::PlatformSharedMemoryRegion* region, subtle::PlatformSharedMemoryRegion* region,
......
...@@ -409,10 +409,6 @@ void NaClProcessHost::Launch( ...@@ -409,10 +409,6 @@ void NaClProcessHost::Launch(
} }
} }
// Create a shared memory region that the renderer and plugin share for
// reporting crash information.
crash_info_shmem_.CreateAnonymous(kNaClCrashInfoShmemSize);
// Launch the process // Launch the process
if (!LaunchSelLdr()) { if (!LaunchSelLdr()) {
delete this; delete this;
...@@ -629,20 +625,19 @@ void NaClProcessHost::OnResourcesReady() { ...@@ -629,20 +625,19 @@ void NaClProcessHost::OnResourcesReady() {
void NaClProcessHost::ReplyToRenderer( void NaClProcessHost::ReplyToRenderer(
mojo::ScopedMessagePipeHandle ppapi_channel_handle, mojo::ScopedMessagePipeHandle ppapi_channel_handle,
mojo::ScopedMessagePipeHandle trusted_channel_handle, mojo::ScopedMessagePipeHandle trusted_channel_handle,
mojo::ScopedMessagePipeHandle manifest_service_channel_handle) { mojo::ScopedMessagePipeHandle manifest_service_channel_handle,
base::ReadOnlySharedMemoryRegion crash_info_shmem_region) {
// Hereafter, we always send an IPC message with handles created above // Hereafter, we always send an IPC message with handles created above
// which, on Windows, are not closable in this process. // which, on Windows, are not closable in this process.
std::string error_message; std::string error_message;
base::SharedMemoryHandle crash_info_shmem_renderer_handle = if (!uses_nonsfi_mode_ && !crash_info_shmem_region.IsValid()) {
crash_info_shmem_.handle().Duplicate();
if (!crash_info_shmem_renderer_handle.IsValid()) {
// On error, we do not send "IPC::ChannelHandle"s to the renderer process. // On error, we do not send "IPC::ChannelHandle"s to the renderer process.
// Note that some other FDs/handles still get sent to the renderer, but // Note that some other FDs/handles still get sent to the renderer, but
// will be closed there. // will be closed there.
ppapi_channel_handle.reset(); ppapi_channel_handle.reset();
trusted_channel_handle.reset(); trusted_channel_handle.reset();
manifest_service_channel_handle.reset(); manifest_service_channel_handle.reset();
error_message = "handle duplication failed"; error_message = "shared memory region not valid";
} }
const ChildProcessData& data = process_->GetData(); const ChildProcessData& data = process_->GetData();
...@@ -650,12 +645,8 @@ void NaClProcessHost::ReplyToRenderer( ...@@ -650,12 +645,8 @@ void NaClProcessHost::ReplyToRenderer(
NaClLaunchResult( NaClLaunchResult(
ppapi_channel_handle.release(), trusted_channel_handle.release(), ppapi_channel_handle.release(), trusted_channel_handle.release(),
manifest_service_channel_handle.release(), data.GetProcess().Pid(), manifest_service_channel_handle.release(), data.GetProcess().Pid(),
data.id, crash_info_shmem_renderer_handle), data.id, std::move(crash_info_shmem_region)),
error_message); error_message);
// Now that the crash information shmem handles have been shared with the
// plugin and the renderer, the browser can close its handle.
crash_info_shmem_.Close();
} }
void NaClProcessHost::SendErrorToRenderer(const std::string& error_message) { void NaClProcessHost::SendErrorToRenderer(const std::string& error_message) {
...@@ -797,9 +788,12 @@ bool NaClProcessHost::StartNaClExecution() { ...@@ -797,9 +788,12 @@ bool NaClProcessHost::StartNaClExecution() {
#endif #endif
} }
params.crash_info_shmem_handle = crash_info_shmem_.handle().Duplicate(); // Create a shared memory region that the renderer and the plugin share to
if (!params.crash_info_shmem_handle.IsValid()) { // report crash information.
DLOG(ERROR) << "Failed to duplicate a shared memory buffer"; params.crash_info_shmem_region =
base::WritableSharedMemoryRegion::Create(kNaClCrashInfoShmemSize);
if (!params.crash_info_shmem_region.IsValid()) {
DLOG(ERROR) << "Failed to create a shared memory buffer";
return false; return false;
} }
...@@ -832,14 +826,16 @@ bool NaClProcessHost::StartNaClExecution() { ...@@ -832,14 +826,16 @@ bool NaClProcessHost::StartNaClExecution() {
// into the plugin process. // into the plugin process.
base::PostTaskWithTraitsAndReplyWithResult( base::PostTaskWithTraitsAndReplyWithResult(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT}, FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::Bind(OpenNaClReadExecImpl, file_path, true /* is_executable */), base::BindOnce(OpenNaClReadExecImpl, file_path,
base::Bind(&NaClProcessHost::StartNaClFileResolved, true /* is_executable */),
weak_factory_.GetWeakPtr(), params, file_path)); base::BindOnce(&NaClProcessHost::StartNaClFileResolved,
weak_factory_.GetWeakPtr(), std::move(params),
file_path));
return true; return true;
} }
} }
StartNaClFileResolved(params, base::FilePath(), base::File()); StartNaClFileResolved(std::move(params), base::FilePath(), base::File());
return true; return true;
} }
...@@ -878,10 +874,12 @@ void NaClProcessHost::StartNaClFileResolved( ...@@ -878,10 +874,12 @@ void NaClProcessHost::StartNaClFileResolved(
// On success, send back a success message to the renderer process, // On success, send back a success message to the renderer process,
// and transfer the channel handles for the NaCl loader process to // and transfer the channel handles for the NaCl loader process to
// |params|. // |params|. Also send an invalid shared memory region as nonsfi_mode
// does not use the region.
ReplyToRenderer(std::move(ppapi_renderer_channel.handle1), ReplyToRenderer(std::move(ppapi_renderer_channel.handle1),
std::move(trusted_service_channel.handle1), std::move(trusted_service_channel.handle1),
std::move(manifest_service_channel.handle1)); std::move(manifest_service_channel.handle1),
base::ReadOnlySharedMemoryRegion());
params.ppapi_browser_channel_handle = params.ppapi_browser_channel_handle =
ppapi_browser_channel.handle0.release(); ppapi_browser_channel.handle0.release();
params.ppapi_renderer_channel_handle = params.ppapi_renderer_channel_handle =
...@@ -893,7 +891,7 @@ void NaClProcessHost::StartNaClFileResolved( ...@@ -893,7 +891,7 @@ void NaClProcessHost::StartNaClFileResolved(
} }
#endif #endif
process_->Send(new NaClProcessMsg_Start(params)); process_->Send(new NaClProcessMsg_Start(std::move(params)));
} }
bool NaClProcessHost::StartPPAPIProxy( bool NaClProcessHost::StartPPAPIProxy(
...@@ -965,7 +963,8 @@ void NaClProcessHost::OnPpapiChannelsCreated( ...@@ -965,7 +963,8 @@ void NaClProcessHost::OnPpapiChannelsCreated(
const IPC::ChannelHandle& raw_ppapi_browser_channel_handle, const IPC::ChannelHandle& raw_ppapi_browser_channel_handle,
const IPC::ChannelHandle& raw_ppapi_renderer_channel_handle, const IPC::ChannelHandle& raw_ppapi_renderer_channel_handle,
const IPC::ChannelHandle& raw_trusted_renderer_channel_handle, const IPC::ChannelHandle& raw_trusted_renderer_channel_handle,
const IPC::ChannelHandle& raw_manifest_service_channel_handle) { const IPC::ChannelHandle& raw_manifest_service_channel_handle,
base::ReadOnlySharedMemoryRegion crash_info_shmem_region) {
DCHECK(raw_ppapi_browser_channel_handle.is_mojo_channel_handle()); DCHECK(raw_ppapi_browser_channel_handle.is_mojo_channel_handle());
DCHECK(raw_ppapi_renderer_channel_handle.is_mojo_channel_handle()); DCHECK(raw_ppapi_renderer_channel_handle.is_mojo_channel_handle());
DCHECK(raw_trusted_renderer_channel_handle.is_mojo_channel_handle()); DCHECK(raw_trusted_renderer_channel_handle.is_mojo_channel_handle());
...@@ -988,7 +987,8 @@ void NaClProcessHost::OnPpapiChannelsCreated( ...@@ -988,7 +987,8 @@ void NaClProcessHost::OnPpapiChannelsCreated(
// Let the renderer know that the IPC channels are established. // Let the renderer know that the IPC channels are established.
ReplyToRenderer(std::move(ppapi_renderer_channel_handle), ReplyToRenderer(std::move(ppapi_renderer_channel_handle),
std::move(trusted_renderer_channel_handle), std::move(trusted_renderer_channel_handle),
std::move(manifest_service_channel_handle)); std::move(manifest_service_channel_handle),
std::move(crash_info_shmem_region));
} }
bool NaClProcessHost::StartWithLaunchedProcess() { bool NaClProcessHost::StartWithLaunchedProcess() {
......
...@@ -15,8 +15,8 @@ ...@@ -15,8 +15,8 @@
#include "base/files/file.h" #include "base/files/file.h"
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/read_only_shared_memory_region.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/memory/shared_memory.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/process/process.h" #include "base/process/process.h"
#include "components/nacl/common/nacl_types.h" #include "components/nacl/common/nacl_types.h"
...@@ -144,7 +144,8 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate { ...@@ -144,7 +144,8 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
void ReplyToRenderer( void ReplyToRenderer(
mojo::ScopedMessagePipeHandle ppapi_channel_handle, mojo::ScopedMessagePipeHandle ppapi_channel_handle,
mojo::ScopedMessagePipeHandle trusted_channel_handle, mojo::ScopedMessagePipeHandle trusted_channel_handle,
mojo::ScopedMessagePipeHandle manifest_service_channel_handle); mojo::ScopedMessagePipeHandle manifest_service_channel_handle,
base::ReadOnlySharedMemoryRegion crash_info_shmem_region);
// Sends the reply with error message to the renderer. // Sends the reply with error message to the renderer.
void SendErrorToRenderer(const std::string& error_message); void SendErrorToRenderer(const std::string& error_message);
...@@ -194,7 +195,8 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate { ...@@ -194,7 +195,8 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
const IPC::ChannelHandle& ppapi_browser_channel_handle, const IPC::ChannelHandle& ppapi_browser_channel_handle,
const IPC::ChannelHandle& ppapi_renderer_channel_handle, const IPC::ChannelHandle& ppapi_renderer_channel_handle,
const IPC::ChannelHandle& trusted_renderer_channel_handle, const IPC::ChannelHandle& trusted_renderer_channel_handle,
const IPC::ChannelHandle& manifest_service_channel_handle); const IPC::ChannelHandle& manifest_service_channel_handle,
base::ReadOnlySharedMemoryRegion crash_info_shmem_region);
GURL manifest_url_; GURL manifest_url_;
base::File nexe_file_; base::File nexe_file_;
...@@ -246,10 +248,6 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate { ...@@ -246,10 +248,6 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
// Throttling time in milliseconds for PpapiHostMsg_Keepalive IPCs. // Throttling time in milliseconds for PpapiHostMsg_Keepalive IPCs.
static unsigned keepalive_throttle_interval_milliseconds_; static unsigned keepalive_throttle_interval_milliseconds_;
// Shared memory provided to the plugin and renderer for
// reporting crash information.
base::SharedMemory crash_info_shmem_;
base::WeakPtrFactory<NaClProcessHost> weak_factory_; base::WeakPtrFactory<NaClProcessHost> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(NaClProcessHost); DISALLOW_COPY_AND_ASSIGN(NaClProcessHost);
......
...@@ -44,7 +44,7 @@ IPC_STRUCT_TRAITS_BEGIN(nacl::NaClLaunchResult) ...@@ -44,7 +44,7 @@ IPC_STRUCT_TRAITS_BEGIN(nacl::NaClLaunchResult)
IPC_STRUCT_TRAITS_MEMBER(manifest_service_ipc_channel_handle) IPC_STRUCT_TRAITS_MEMBER(manifest_service_ipc_channel_handle)
IPC_STRUCT_TRAITS_MEMBER(plugin_pid) IPC_STRUCT_TRAITS_MEMBER(plugin_pid)
IPC_STRUCT_TRAITS_MEMBER(plugin_child_id) IPC_STRUCT_TRAITS_MEMBER(plugin_child_id)
IPC_STRUCT_TRAITS_MEMBER(crash_info_shmem_handle) IPC_STRUCT_TRAITS_MEMBER(crash_info_shmem_region)
IPC_STRUCT_TRAITS_END() IPC_STRUCT_TRAITS_END()
IPC_STRUCT_TRAITS_BEGIN(nacl::PnaclCacheInfo) IPC_STRUCT_TRAITS_BEGIN(nacl::PnaclCacheInfo)
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <stdint.h> #include <stdint.h>
#include "base/memory/read_only_shared_memory_region.h"
#include "base/process/process.h" #include "base/process/process.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "components/nacl/common/nacl_types.h" #include "components/nacl/common/nacl_types.h"
...@@ -38,7 +39,7 @@ IPC_STRUCT_TRAITS_BEGIN(nacl::NaClStartParams) ...@@ -38,7 +39,7 @@ IPC_STRUCT_TRAITS_BEGIN(nacl::NaClStartParams)
IPC_STRUCT_TRAITS_MEMBER(version) IPC_STRUCT_TRAITS_MEMBER(version)
IPC_STRUCT_TRAITS_MEMBER(enable_debug_stub) IPC_STRUCT_TRAITS_MEMBER(enable_debug_stub)
IPC_STRUCT_TRAITS_MEMBER(process_type) IPC_STRUCT_TRAITS_MEMBER(process_type)
IPC_STRUCT_TRAITS_MEMBER(crash_info_shmem_handle) IPC_STRUCT_TRAITS_MEMBER(crash_info_shmem_region)
IPC_STRUCT_TRAITS_END() IPC_STRUCT_TRAITS_END()
IPC_STRUCT_TRAITS_BEGIN(nacl::NaClResourcePrefetchResult) IPC_STRUCT_TRAITS_BEGIN(nacl::NaClResourcePrefetchResult)
...@@ -129,8 +130,10 @@ IPC_MESSAGE_CONTROL4(NaClProcessMsg_ResolveFileTokenReply, ...@@ -129,8 +130,10 @@ IPC_MESSAGE_CONTROL4(NaClProcessMsg_ResolveFileTokenReply,
// created successfully. // created successfully.
// This is used for SFI mode only. Non-SFI mode passes channel handles in // This is used for SFI mode only. Non-SFI mode passes channel handles in
// NaClStartParams instead. // NaClStartParams instead.
IPC_MESSAGE_CONTROL4(NaClProcessHostMsg_PpapiChannelsCreated, IPC_MESSAGE_CONTROL5(
IPC::ChannelHandle, /* browser_channel_handle */ NaClProcessHostMsg_PpapiChannelsCreated,
IPC::ChannelHandle, /* ppapi_renderer_channel_handle */ IPC::ChannelHandle, /* browser_channel_handle */
IPC::ChannelHandle, /* trusted_renderer_channel_handle */ IPC::ChannelHandle, /* ppapi_renderer_channel_handle */
IPC::ChannelHandle /* manifest_service_channel_handle */) IPC::ChannelHandle, /* trusted_renderer_channel_handle */
IPC::ChannelHandle, /* manifest_service_channel_handle */
base::ReadOnlySharedMemoryRegion /* crash_info_shmem_region */)
...@@ -19,7 +19,7 @@ NaClStartParams::NaClStartParams() ...@@ -19,7 +19,7 @@ NaClStartParams::NaClStartParams()
process_type(kUnknownNaClProcessType) { process_type(kUnknownNaClProcessType) {
} }
NaClStartParams::NaClStartParams(const NaClStartParams& other) = default; NaClStartParams::NaClStartParams(NaClStartParams&& other) = default;
NaClStartParams::~NaClStartParams() { NaClStartParams::~NaClStartParams() {
} }
...@@ -98,14 +98,13 @@ NaClLaunchResult::NaClLaunchResult( ...@@ -98,14 +98,13 @@ NaClLaunchResult::NaClLaunchResult(
const IPC::ChannelHandle& manifest_service_ipc_channel_handle, const IPC::ChannelHandle& manifest_service_ipc_channel_handle,
base::ProcessId plugin_pid, base::ProcessId plugin_pid,
int plugin_child_id, int plugin_child_id,
base::SharedMemoryHandle crash_info_shmem_handle) base::ReadOnlySharedMemoryRegion crash_info_shmem_region)
: ppapi_ipc_channel_handle(ppapi_ipc_channel_handle), : ppapi_ipc_channel_handle(ppapi_ipc_channel_handle),
trusted_ipc_channel_handle(trusted_ipc_channel_handle), trusted_ipc_channel_handle(trusted_ipc_channel_handle),
manifest_service_ipc_channel_handle(manifest_service_ipc_channel_handle), manifest_service_ipc_channel_handle(manifest_service_ipc_channel_handle),
plugin_pid(plugin_pid), plugin_pid(plugin_pid),
plugin_child_id(plugin_child_id), plugin_child_id(plugin_child_id),
crash_info_shmem_handle(crash_info_shmem_handle) { crash_info_shmem_region(std::move(crash_info_shmem_region)) {}
}
NaClLaunchResult::~NaClLaunchResult() { NaClLaunchResult::~NaClLaunchResult() {
} }
......
...@@ -11,7 +11,8 @@ ...@@ -11,7 +11,8 @@
#include <utility> #include <utility>
#include <vector> #include <vector>
#include "base/memory/shared_memory.h" #include "base/memory/read_only_shared_memory_region.h"
#include "base/memory/writable_shared_memory_region.h"
#include "base/process/process_handle.h" #include "base/process/process_handle.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "ipc/ipc_channel.h" #include "ipc/ipc_channel.h"
...@@ -67,7 +68,7 @@ struct NaClResourcePrefetchResult { ...@@ -67,7 +68,7 @@ struct NaClResourcePrefetchResult {
// Parameters sent to the NaCl process when we start it. // Parameters sent to the NaCl process when we start it.
struct NaClStartParams { struct NaClStartParams {
NaClStartParams(); NaClStartParams();
NaClStartParams(const NaClStartParams& other); NaClStartParams(NaClStartParams&& other);
~NaClStartParams(); ~NaClStartParams();
IPC::PlatformFileForTransit nexe_file; IPC::PlatformFileForTransit nexe_file;
...@@ -103,11 +104,14 @@ struct NaClStartParams { ...@@ -103,11 +104,14 @@ struct NaClStartParams {
NaClAppProcessType process_type; NaClAppProcessType process_type;
// For NaCl <-> renderer crash information reporting. // For NaCl <-> renderer crash information reporting.
base::SharedMemoryHandle crash_info_shmem_handle; base::WritableSharedMemoryRegion crash_info_shmem_region;
// NOTE: Any new fields added here must also be added to the IPC // NOTE: Any new fields added here must also be added to the IPC
// serialization in nacl_messages.h and (for POD fields) the constructor // serialization in nacl_messages.h and (for POD fields) the constructor
// in nacl_types.cc. // in nacl_types.cc.
private:
DISALLOW_COPY_AND_ASSIGN(NaClStartParams);
}; };
// Parameters sent to the browser process to have it launch a NaCl process. // Parameters sent to the browser process to have it launch a NaCl process.
...@@ -153,7 +157,7 @@ struct NaClLaunchResult { ...@@ -153,7 +157,7 @@ struct NaClLaunchResult {
const IPC::ChannelHandle& manifest_service_ipc_channel_handle, const IPC::ChannelHandle& manifest_service_ipc_channel_handle,
base::ProcessId plugin_pid, base::ProcessId plugin_pid,
int plugin_child_id, int plugin_child_id,
base::SharedMemoryHandle crash_info_shmem_handle); base::ReadOnlySharedMemoryRegion crash_info_shmem_region);
~NaClLaunchResult(); ~NaClLaunchResult();
// For plugin <-> renderer PPAPI communication. // For plugin <-> renderer PPAPI communication.
...@@ -170,7 +174,10 @@ struct NaClLaunchResult { ...@@ -170,7 +174,10 @@ struct NaClLaunchResult {
int plugin_child_id; int plugin_child_id;
// For NaCl <-> renderer crash information reporting. // For NaCl <-> renderer crash information reporting.
base::SharedMemoryHandle crash_info_shmem_handle; base::ReadOnlySharedMemoryRegion crash_info_shmem_region;
private:
DISALLOW_COPY_AND_ASSIGN(NaClLaunchResult);
}; };
} // namespace nacl } // namespace nacl
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include "base/command_line.h" #include "base/command_line.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/memory/read_only_shared_memory_region.h"
#include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop.h"
#include "base/rand_util.h" #include "base/rand_util.h"
#include "base/run_loop.h" #include "base/run_loop.h"
...@@ -282,7 +283,7 @@ void NaClListener::OnAddPrefetchedResource( ...@@ -282,7 +283,7 @@ void NaClListener::OnAddPrefetchedResource(
} }
} }
void NaClListener::OnStart(const nacl::NaClStartParams& params) { void NaClListener::OnStart(nacl::NaClStartParams params) {
is_started_ = true; is_started_ = true;
#if defined(OS_LINUX) || defined(OS_MACOSX) #if defined(OS_LINUX) || defined(OS_MACOSX)
int urandom_fd = HANDLE_EINTR(dup(base::GetUrandomFD())); int urandom_fd = HANDLE_EINTR(dup(base::GetUrandomFD()));
...@@ -294,10 +295,13 @@ void NaClListener::OnStart(const nacl::NaClStartParams& params) { ...@@ -294,10 +295,13 @@ void NaClListener::OnStart(const nacl::NaClStartParams& params) {
struct NaClApp* nap = NULL; struct NaClApp* nap = NULL;
NaClChromeMainInit(); NaClChromeMainInit();
CHECK(base::SharedMemory::IsHandleValid(params.crash_info_shmem_handle)); CHECK(params.crash_info_shmem_region.IsValid());
crash_info_shmem_.reset(new base::SharedMemory( crash_info_shmem_mapping_ = params.crash_info_shmem_region.Map();
params.crash_info_shmem_handle, false /* not readonly */)); base::ReadOnlySharedMemoryRegion ro_shmem_region =
CHECK(crash_info_shmem_->Map(nacl::kNaClCrashInfoShmemSize)); base::WritableSharedMemoryRegion::ConvertToReadOnly(
std::move(params.crash_info_shmem_region));
CHECK(crash_info_shmem_mapping_.IsValid());
CHECK(ro_shmem_region.IsValid());
NaClSetFatalErrorCallback(&FatalLogHandler); NaClSetFatalErrorCallback(&FatalLogHandler);
nap = NaClAppCreate(); nap = NaClAppCreate();
...@@ -330,7 +334,7 @@ void NaClListener::OnStart(const nacl::NaClStartParams& params) { ...@@ -330,7 +334,7 @@ void NaClListener::OnStart(const nacl::NaClStartParams& params) {
if (!Send(new NaClProcessHostMsg_PpapiChannelsCreated( if (!Send(new NaClProcessHostMsg_PpapiChannelsCreated(
browser_handle, ppapi_renderer_handle, browser_handle, ppapi_renderer_handle,
MakeRequest(&renderer_host).PassMessagePipe().release(), MakeRequest(&renderer_host).PassMessagePipe().release(),
manifest_service_handle))) manifest_service_handle, ro_shmem_region)))
LOG(FATAL) << "Failed to send IPC channel handle to NaClProcessHost."; LOG(FATAL) << "Failed to send IPC channel handle to NaClProcessHost.";
trusted_listener_ = std::make_unique<NaClTrustedListener>( trusted_listener_ = std::make_unique<NaClTrustedListener>(
......
...@@ -14,7 +14,7 @@ ...@@ -14,7 +14,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/memory/shared_memory.h" #include "base/memory/shared_memory_mapping.h"
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "base/synchronization/waitable_event.h" #include "base/synchronization/waitable_event.h"
#include "base/threading/thread.h" #include "base/threading/thread.h"
...@@ -50,7 +50,9 @@ class NaClListener : public IPC::Listener { ...@@ -50,7 +50,9 @@ class NaClListener : public IPC::Listener {
} }
#endif #endif
void* crash_info_shmem_memory() const { return crash_info_shmem_->memory(); } void* crash_info_shmem_memory() const {
return crash_info_shmem_mapping_.memory();
}
NaClTrustedListener* trusted_listener() const { NaClTrustedListener* trusted_listener() const {
return trusted_listener_.get(); return trusted_listener_.get();
...@@ -79,7 +81,7 @@ class NaClListener : public IPC::Listener { ...@@ -79,7 +81,7 @@ class NaClListener : public IPC::Listener {
void OnAddPrefetchedResource( void OnAddPrefetchedResource(
const nacl::NaClResourcePrefetchResult& prefetched_resource_file); const nacl::NaClResourcePrefetchResult& prefetched_resource_file);
void OnStart(const nacl::NaClStartParams& params); void OnStart(nacl::NaClStartParams params);
// A channel back to the browser. // A channel back to the browser.
std::unique_ptr<IPC::SyncChannel> channel_; std::unique_ptr<IPC::SyncChannel> channel_;
...@@ -102,7 +104,7 @@ class NaClListener : public IPC::Listener { ...@@ -102,7 +104,7 @@ class NaClListener : public IPC::Listener {
int number_of_cores_; int number_of_cores_;
#endif #endif
std::unique_ptr<base::SharedMemory> crash_info_shmem_; base::WritableSharedMemoryMapping crash_info_shmem_mapping_;
std::unique_ptr<NaClTrustedListener> trusted_listener_; std::unique_ptr<NaClTrustedListener> trusted_listener_;
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/command_line.h" #include "base/command_line.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/memory/shared_memory_mapping.h"
#include "base/metrics/histogram.h" #include "base/metrics/histogram.h"
#include "base/strings/string_tokenizer.h" #include "base/strings/string_tokenizer.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
...@@ -101,8 +102,6 @@ NexeLoadManager::~NexeLoadManager() { ...@@ -101,8 +102,6 @@ NexeLoadManager::~NexeLoadManager() {
base::TimeDelta uptime = base::Time::Now() - ready_time_; base::TimeDelta uptime = base::Time::Now() - ready_time_;
HistogramTimeLarge("NaCl.ModuleUptime.Normal", uptime.InMilliseconds()); HistogramTimeLarge("NaCl.ModuleUptime.Normal", uptime.InMilliseconds());
} }
if (base::SharedMemory::IsHandleValid(crash_info_shmem_handle_))
base::SharedMemory::CloseHandle(crash_info_shmem_handle_);
} }
void NexeLoadManager::NexeFileDidOpen(int32_t pp_error, void NexeLoadManager::NexeFileDidOpen(int32_t pp_error,
...@@ -260,23 +259,15 @@ void NexeLoadManager::NexeDidCrash() { ...@@ -260,23 +259,15 @@ void NexeLoadManager::NexeDidCrash() {
// invocation will just be a no-op, since the entire crash log will // invocation will just be a no-op, since the entire crash log will
// have been received and we'll just get an EOF indication. // have been received and we'll just get an EOF indication.
base::SharedMemory shmem(crash_info_shmem_handle_, true); base::ReadOnlySharedMemoryMapping shmem_mapping =
// When shmem goes out of scope, the handle will be closed. Invalidate crash_info_shmem_region_.MapAt(0, kNaClCrashInfoShmemSize);
// our handle so our destructor doesn't try to close it again. if (shmem_mapping.IsValid()) {
crash_info_shmem_handle_ = base::SharedMemoryHandle(); base::BufferIterator<const uint8_t> buffer =
if (shmem.Map(kNaClCrashInfoShmemSize)) { shmem_mapping.GetMemoryAsBufferIterator<uint8_t>();
uint32_t crash_log_length; const uint32_t* crash_log_length = buffer.Object<uint32_t>();
// We cast the length value to volatile here to prevent the compiler from base::span<const uint8_t> data = buffer.Span<uint8_t>(
// reordering instructions in a way that could introduce a TOCTTOU race. std::min<uint32_t>(*crash_log_length, kNaClCrashInfoMaxLogSize));
crash_log_length = *(static_cast<volatile uint32_t*>(shmem.memory())); std::string crash_log(data.begin(), data.end());
crash_log_length = std::min<uint32_t>(crash_log_length,
kNaClCrashInfoMaxLogSize);
std::unique_ptr<char[]> crash_log_data(new char[kNaClCrashInfoShmemSize]);
memcpy(crash_log_data.get(),
static_cast<char*>(shmem.memory()) + sizeof(uint32_t),
crash_log_length);
std::string crash_log(crash_log_data.get(), crash_log_length);
CopyCrashLogToJsConsole(crash_log); CopyCrashLogToJsConsole(crash_log);
} }
} }
......
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
#include "base/files/file.h" #include "base/files/file.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/shared_memory.h" #include "base/memory/read_only_shared_memory_region.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "components/nacl/renderer/ppb_nacl_private.h" #include "components/nacl/renderer/ppb_nacl_private.h"
...@@ -115,8 +115,9 @@ class NexeLoadManager { ...@@ -115,8 +115,9 @@ class NexeLoadManager {
const std::string& program_url() const { return program_url_; } const std::string& program_url() const { return program_url_; }
void set_crash_info_shmem_handle(base::SharedMemoryHandle h) { void set_crash_info_shmem_region(
crash_info_shmem_handle_ = h; base::ReadOnlySharedMemoryRegion shmem_region) {
crash_info_shmem_region_ = std::move(shmem_region);
} }
bool nonsfi() const { return nonsfi_; } bool nonsfi() const { return nonsfi_; }
...@@ -184,7 +185,7 @@ class NexeLoadManager { ...@@ -184,7 +185,7 @@ class NexeLoadManager {
// A flag that indicates if the plugin is using Non-SFI mode. // A flag that indicates if the plugin is using Non-SFI mode.
bool nonsfi_; bool nonsfi_;
base::SharedMemoryHandle crash_info_shmem_handle_; base::ReadOnlySharedMemoryRegion crash_info_shmem_region_;
std::unique_ptr<TrustedPluginChannel> trusted_plugin_channel_; std::unique_ptr<TrustedPluginChannel> trusted_plugin_channel_;
std::unique_ptr<ManifestServiceChannel> manifest_service_channel_; std::unique_ptr<ManifestServiceChannel> manifest_service_channel_;
......
...@@ -487,10 +487,6 @@ void PPBNaClPrivate::LaunchSelLdr( ...@@ -487,10 +487,6 @@ void PPBNaClPrivate::LaunchSelLdr(
// Even on error, some FDs/handles may be passed to here. // Even on error, some FDs/handles may be passed to here.
// We must release those resources. // We must release those resources.
// See also nacl_process_host.cc. // See also nacl_process_host.cc.
if (base::SharedMemory::IsHandleValid(
launch_result.crash_info_shmem_handle))
base::SharedMemory::CloseHandle(launch_result.crash_info_shmem_handle);
if (PP_ToBool(main_service_runtime)) { if (PP_ToBool(main_service_runtime)) {
load_manager->ReportLoadError(PP_NACL_ERROR_SEL_LDR_LAUNCH, load_manager->ReportLoadError(PP_NACL_ERROR_SEL_LDR_LAUNCH,
"ServiceRuntime: failed to start", "ServiceRuntime: failed to start",
...@@ -527,8 +523,8 @@ void PPBNaClPrivate::LaunchSelLdr( ...@@ -527,8 +523,8 @@ void PPBNaClPrivate::LaunchSelLdr(
} }
// Store the crash information shared memory handle. // Store the crash information shared memory handle.
load_manager->set_crash_info_shmem_handle( load_manager->set_crash_info_shmem_region(
launch_result.crash_info_shmem_handle); std::move(launch_result.crash_info_shmem_region));
// Create the trusted plugin channel. // Create the trusted plugin channel.
if (!IsValidChannelHandle(launch_result.trusted_ipc_channel_handle)) { if (!IsValidChannelHandle(launch_result.trusted_ipc_channel_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