Commit 60e1e0e9 authored by David 'Digit' Turner's avatar David 'Digit' Turner Committed by Commit Bot

Disable memfd base::SharedMemory implementation.

Since there is no way to make read-only file descriptors from Linux
memfd-based regions, the security guarantees of GetReadOnlyHandle()
cannot be maintained with this implementation.

Remove it to fall-back on traditional Posix shared regions instead.

Bug: 792117, 736452
Change-Id: Ie5eb41fc3c4dd02ebdbb77be8375363ba51f1b00
Reviewed-on: https://chromium-review.googlesource.com/809014Reviewed-by: default avatarMark Mentovai <mark@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Commit-Queue: David Turner <digit@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521768}
parent 4bff19f2
......@@ -27,10 +27,6 @@
#include "base/unguessable_token.h"
#include "build/build_config.h"
#if defined(OS_LINUX)
#include <sys/syscall.h>
#endif
#if defined(OS_ANDROID)
#include "base/os_compat_android.h"
#include "third_party/ashmem/ashmem.h"
......@@ -86,42 +82,6 @@ bool SharedMemory::CreateAndMapAnonymous(size_t size) {
#if !defined(OS_ANDROID)
// TODO(crbug.com/789031): Remove #if once all ChromeOS builds support it.
#if defined(__NR_memfd_create)
bool CreateMemFDSharedMemory(const SharedMemoryCreateOptions& options,
ScopedFD* fd,
ScopedFD* readonly_fd,
FilePath* path) {
static bool try_memfd_create = true;
// Avoid sending unnecessary system calls that are not supported.
// There's not guarantee of only trying this once, but the result should
// always be the same when the memfd_create call is not available.
if (!try_memfd_create)
return false;
fd->reset(syscall(__NR_memfd_create, path->BaseName().value().c_str(), 0));
if (!fd->is_valid()) {
if (errno == ENOSYS)
try_memfd_create = false;
else
DPLOG(ERROR) << "memfd(create) failed";
return false;
}
if (options.share_read_only) {
// memfd anonymous files do not support dropping write access for a
// single fd.
int fd_read_only = HANDLE_EINTR(dup(fd->get()));
readonly_fd->reset(fd_read_only);
if (!readonly_fd->is_valid()) {
DPLOG(ERROR) << "memfd(duplicate) failed";
fd->reset();
return false;
}
}
return true;
}
#endif // defined(__NR_memfd_create)
// Chromium mostly only uses 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
......@@ -145,12 +105,8 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
ScopedFD readonly_fd;
FilePath path;
if (!options.name_deprecated || options.name_deprecated->empty()) {
bool result = false;
#if defined(__NR_memfd_create)
result = CreateMemFDSharedMemory(options, &fd, &readonly_fd, &path);
#endif // defined(__NR_memfd_create)
if (!result)
result = CreateAnonymousSharedMemory(options, &fd, &readonly_fd, &path);
bool result =
CreateAnonymousSharedMemory(options, &fd, &readonly_fd, &path);
if (!result)
return false;
} else {
......
......@@ -373,12 +373,9 @@ TEST(SharedMemoryTest, GetReadOnlyHandle) {
// pipe would transform it into read/write.
SharedMemoryHandle handle = readonly_shmem.handle();
#if defined(OS_ANDROID) || defined(__NR_memfd_create)
#if defined(OS_ANDROID)
// The "read-only" handle is still writable on Android:
// http://crbug.com/320865
// When __NR_memfd_create is defined at build time, it indicates that the
// memfd_create syscall is most likely available. In this case there's not
// support for read only handles.
(void)handle;
#elif defined(OS_FUCHSIA)
uintptr_t addr;
......@@ -575,10 +572,8 @@ TEST(SharedMemoryTest, AnonymousExecutable) {
// shared memory implementation. So the tests about file permissions are not
// included on Android. Fuchsia does not use a file-backed shared memory
// implementation.
// In OS_LINUX, when memfd_create syscall is available (__NR_memfd_create is
// defined) SharedMemory doesn't use files in its implementation.
#if !defined(OS_ANDROID) && !defined(OS_FUCHSIA) && !defined(__NR_memfd_create)
#if !defined(OS_ANDROID) && !defined(OS_FUCHSIA)
// Set a umask and restore the old mask on destruction.
class ScopedUmaskSetter {
......@@ -643,8 +638,7 @@ TEST(SharedMemoryTest, FilePermissionsNamed) {
EXPECT_FALSE(shm_stat.st_mode & S_IRWXO);
EXPECT_FALSE(shm_stat.st_mode & S_IRWXG);
}
#endif // !defined(OS_ANDROID) && !defined(OS_FUCHSIA) &&
// !defined(__NR_memfd_create)
#endif // !defined(OS_ANDROID) && !defined(OS_FUCHSIA)
#endif // defined(OS_POSIX)
......
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