Bug 1440199 - Part 2: Remove named mode from IPC shared memory. r=froydnj

We're not using named shared memory, and supporting only anonymous
shared memory allows using other backends that are more compatible
with preventing a process from accessing any shared memory it wasn't
explicitly granted (i.e., sandboxing).

Specifically: SharedMemory::Open is removed; SharedMemory::Create no
longer takes a name, no longer has the open_existing option which doesn't
apply to anonymous memory, and no longer supports read-only memory
(anonymous memory which can never have been written isn't very useful).

This patch also fixes some comments in what remains of SharedMemory::Create.

MozReview-Commit-ID: 4kBrURtxq20

--HG--
extra : rebase_source : f6b1fb2fc79b6e9cdd251b3d9041036c0be503f9
This commit is contained in:
Jed Davis 2018-02-20 13:07:32 -07:00
parent 444639dc96
commit 91efa87e62
6 changed files with 31 additions and 183 deletions

View File

@ -2013,8 +2013,7 @@ ContentParent::LaunchSubprocess(ProcessPriority aInitialPriority /* = PROCESS_PR
// Set up the shared memory.
base::SharedMemory shm;
if (!shm.Create("", /* read_only */ false, /* open_existing */ false,
prefs.Length())) {
if (!shm.Create(prefs.Length())) {
NS_ERROR("failed to create shared memory in the parent");
MarkAsDead();
return false;

View File

@ -25,7 +25,7 @@ SharedDIB::Create(uint32_t aSize)
Close();
mShMem = new base::SharedMemory();
if (!mShMem || !mShMem->Create("", false, false, aSize))
if (!mShMem || !mShMem->Create(aSize))
return NS_ERROR_OUT_OF_MEMORY;
return NS_OK;

View File

@ -60,24 +60,9 @@ class SharedMemory {
// Return invalid handle (see comment above for exact definition).
static SharedMemoryHandle NULLHandle();
// Creates or opens a shared memory segment based on a name.
// If read_only is true, opens the memory as read-only.
// If open_existing is true, and the shared memory already exists,
// opens the existing shared memory and ignores the size parameter.
// If name is the empty string, use a unique name.
// Creates a shared memory segment.
// Returns true on success, false on failure.
bool Create(const std::string& name, bool read_only, bool open_existing,
size_t size);
// Deletes resources associated with a shared memory segment based on name.
// Not all platforms require this call.
bool Delete(const std::wstring& name);
// Opens a shared memory segment based on a name.
// If read_only is true, opens for read-only access.
// If name is the empty string, use a unique name.
// Returns true on success, false on failure.
bool Open(const std::wstring& name, bool read_only);
bool Create(size_t size);
// Maps the shared memory into the caller's address space.
// Returns true on success, false otherwise. The memory address
@ -140,17 +125,11 @@ class SharedMemory {
}
private:
#if defined(OS_POSIX)
bool CreateOrOpen(const std::wstring &name, int posix_flags, size_t size);
bool FilenameForMemoryName(const std::wstring &memname,
std::wstring *filename);
#endif
bool ShareToProcessCommon(ProcessId target_pid,
SharedMemoryHandle* new_handle,
bool close_self);
#if defined(OS_WIN)
std::wstring name_;
HANDLE mapped_file_;
#elif defined(OS_POSIX)
int mapped_file_;

View File

@ -56,72 +56,6 @@ SharedMemoryHandle SharedMemory::NULLHandle() {
return SharedMemoryHandle();
}
bool SharedMemory::Create(const std::string &cname, bool read_only,
bool open_existing, size_t size) {
read_only_ = read_only;
std::wstring name = UTF8ToWide(cname);
int posix_flags = 0;
posix_flags |= read_only ? O_RDONLY : O_RDWR;
if (!open_existing || mapped_file_ <= 0)
posix_flags |= O_CREAT;
if (!CreateOrOpen(name, posix_flags, size))
return false;
max_size_ = size;
return true;
}
// Our current implementation of shmem is with mmap()ing of files.
// These files need to be deleted explicitly.
// In practice this call is only needed for unit tests.
bool SharedMemory::Delete(const std::wstring& name) {
std::wstring mem_filename;
if (FilenameForMemoryName(name, &mem_filename) == false)
return false;
FilePath path(WideToUTF8(mem_filename));
if (file_util::PathExists(path)) {
return file_util::Delete(path);
}
// Doesn't exist, so success.
return true;
}
bool SharedMemory::Open(const std::wstring &name, bool read_only) {
read_only_ = read_only;
int posix_flags = 0;
posix_flags |= read_only ? O_RDONLY : O_RDWR;
return CreateOrOpen(name, posix_flags, 0);
}
// For the given shmem named |memname|, return a filename to mmap()
// (and possibly create). Modifies |filename|. Return false on
// error, or true of we are happy.
bool SharedMemory::FilenameForMemoryName(const std::wstring &memname,
std::wstring *filename) {
std::wstring mem_filename;
// mem_name will be used for a filename; make sure it doesn't
// contain anything which will confuse us.
DCHECK(memname.find_first_of(L"/") == std::string::npos);
DCHECK(memname.find_first_of(L"\0") == std::string::npos);
FilePath temp_dir;
if (file_util::GetShmemTempDir(&temp_dir) == false)
return false;
mem_filename = UTF8ToWide(temp_dir.value());
file_util::AppendToPath(&mem_filename, L"com.google.chrome.shmem." + memname);
*filename = mem_filename;
return true;
}
namespace {
// A class to handle auto-closing of FILE*'s.
@ -138,75 +72,39 @@ typedef mozilla::UniquePtr<FILE, ScopedFILEClose> ScopedFILE;
}
// Chromium mostly only use the unique/private shmem as specified by
// "name == L"". The exception is in the StatsTable.
// TODO(jrg): there is no way to "clean up" all unused named shmem if
// we restart from a crash. (That isn't a new problem, but it is a problem.)
// In case we want to delete it later, it may be useful to save the value
// of mem_filename after FilenameForMemoryName().
bool SharedMemory::CreateOrOpen(const std::wstring &name,
int posix_flags, size_t size) {
bool SharedMemory::Create(size_t size) {
read_only_ = false;
DCHECK(size > 0);
DCHECK(mapped_file_ == -1);
ScopedFILE file_closer;
FILE *fp;
if (name == L"") {
// It doesn't make sense to have a read-only private piece of shmem
DCHECK(posix_flags & (O_RDWR | O_WRONLY));
FilePath path;
fp = file_util::CreateAndOpenTemporaryShmemFile(&path);
FilePath path;
fp = file_util::CreateAndOpenTemporaryShmemFile(&path);
// Deleting the file prevents anyone else from mapping it in
// (making it private), and prevents the need for cleanup (once
// the last fd is closed, it is truly freed).
file_util::Delete(path);
} else {
std::wstring mem_filename;
if (FilenameForMemoryName(name, &mem_filename) == false)
return false;
std::string mode;
switch (posix_flags) {
case (O_RDWR | O_CREAT):
// Careful: "w+" will truncate if it already exists.
mode = "a+";
break;
case O_RDWR:
mode = "r+";
break;
case O_RDONLY:
mode = "r";
break;
default:
NOTIMPLEMENTED();
break;
}
fp = file_util::OpenFile(mem_filename, mode.c_str());
}
// Deleting the file prevents anyone else from mapping it in
// (making it private), and prevents the need for cleanup (once
// the last fd is closed, it is truly freed).
file_util::Delete(path);
if (fp == NULL)
return false;
file_closer.reset(fp); // close when we go out of scope
// Make sure the (new) file is the right size.
// According to the man page, "Use of truncate() to extend a file is
// not portable."
if (size && (posix_flags & (O_RDWR | O_CREAT))) {
// Get current size.
struct stat stat;
if (fstat(fileno(fp), &stat) != 0)
return false;
size_t current_size = stat.st_size;
if (current_size != size) {
if (ftruncate(fileno(fp), size) != 0)
return false;
if (fseeko(fp, size, SEEK_SET) != 0)
return false;
}
}
// Set the file size.
//
// According to POSIX, (f)truncate can be used to extend files;
// previously this required the XSI option but as of the 2008
// edition it's required for everything. (Linux documents that this
// may fail on non-"native" filesystems like FAT, but /dev/shm
// should always be tmpfs.)
if (ftruncate(fileno(fp), size) != 0)
return false;
// This probably isn't needed.
if (fseeko(fp, size, SEEK_SET) != 0)
return false;
mapped_file_ = dup(fileno(fp));
DCHECK(mapped_file_ >= 0);
@ -216,6 +114,7 @@ bool SharedMemory::CreateOrOpen(const std::wstring &name,
NOTREACHED();
inode_ = st.st_ino;
max_size_ = size;
return true;
}

View File

@ -42,47 +42,18 @@ SharedMemoryHandle SharedMemory::NULLHandle() {
return NULL;
}
bool SharedMemory::Create(const std::string &cname, bool read_only,
bool open_existing, size_t size) {
bool SharedMemory::Create(size_t size) {
DCHECK(mapped_file_ == NULL);
std::wstring name = UTF8ToWide(cname);
name_ = name;
read_only_ = read_only;
read_only_ = false;
mapped_file_ = CreateFileMapping(INVALID_HANDLE_VALUE, NULL,
read_only_ ? PAGE_READONLY : PAGE_READWRITE, 0, static_cast<DWORD>(size),
name.empty() ? NULL : name.c_str());
PAGE_READWRITE, 0, static_cast<DWORD>(size), NULL);
if (!mapped_file_)
return false;
// Check if the shared memory pre-exists.
if (GetLastError() == ERROR_ALREADY_EXISTS && !open_existing) {
Close();
return false;
}
max_size_ = size;
return true;
}
bool SharedMemory::Delete(const std::wstring& name) {
// intentionally empty -- there is nothing for us to do on Windows.
return true;
}
bool SharedMemory::Open(const std::wstring &name, bool read_only) {
DCHECK(mapped_file_ == NULL);
name_ = name;
read_only_ = read_only;
mapped_file_ = OpenFileMapping(
read_only_ ? FILE_MAP_READ : FILE_MAP_ALL_ACCESS, false,
name.empty() ? NULL : name.c_str());
if (mapped_file_ != NULL) {
// Note: size_ is not set in this case.
return true;
}
return false;
}
bool SharedMemory::Map(size_t bytes) {
if (mapped_file_ == NULL)
return false;

View File

@ -37,7 +37,7 @@ public:
virtual bool Create(size_t aNbytes) override
{
bool ok = mSharedMemory.Create("", false, false, aNbytes);
bool ok = mSharedMemory.Create(aNbytes);
if (ok) {
Created(aNbytes);
}