Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(5419)

Unified Diff: base/memory/shared_memory_unittest.cc

Issue 27265002: Implement SharedMemory::NewAnonymousReadOnly(contents). (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Add errno.h Created 7 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: base/memory/shared_memory_unittest.cc
diff --git a/base/memory/shared_memory_unittest.cc b/base/memory/shared_memory_unittest.cc
index 892fd7f1a590b0d20b8f9e6289b07a225c8676fa..c5b4310e8269cbcadd14716670f1f124c3860b81 100644
--- a/base/memory/shared_memory_unittest.cc
+++ b/base/memory/shared_memory_unittest.cc
@@ -8,6 +8,7 @@
#include "base/process/kill.h"
#include "base/rand_util.h"
#include "base/strings/string_number_conversions.h"
+#include "base/strings/stringprintf.h"
#include "base/sys_info.h"
#include "base/test/multiprocess_test.h"
#include "base/threading/platform_thread.h"
@@ -20,6 +21,8 @@
#endif
#if defined(OS_POSIX)
+#include <errno.h>
+#include <fcntl.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/types.h>
@@ -361,6 +364,91 @@ TEST(SharedMemoryTest, AnonymousPrivate) {
}
}
+TEST(SharedMemoryTest, AnonymousReadOnly) {
+ StringPiece contents = "Hello World";
+ scoped_ptr<SharedMemory> shmem(
+ SharedMemory::NewAnonymousReadOnly("Hello World"));
+
+ ASSERT_TRUE(shmem->Map(contents.size()));
+ EXPECT_EQ(
+ contents,
+ StringPiece(static_cast<const char*>(shmem->memory()), contents.size()));
+
+ // We'd like to check that if we send the read-only segment to another
+ // process, then that other process can't reopen it read/write. (Since that
+ // would be a security hole.) Setting up multiple processes is hard in a
+ // unittest, so this test checks that the *current* process can't reopen the
+ // segment read/write. I think the test here is stronger than we actually
+ // care about, but there's a remote possibility that sending a file over a
+ // pipe would transform it into read/write.
+ SharedMemoryHandle handle = shmem->handle();
+#if OS_POSIX
jln (very slow on Chromium) 2013/10/16 00:16:39 Style: #if defined()
Jeffrey Yasskin 2013/10/16 01:16:39 Done.
+
+ EXPECT_EQ(O_RDONLY, fcntl(handle.fd, F_GETFL) & O_ACCMODE)
+ << "The descriptor itself should be read-only.";
+
+ errno = 0;
+ void* writable = mmap(NULL,
+ shmem->mapped_size(),
+ PROT_READ | PROT_WRITE,
+ MAP_SHARED,
+ handle.fd,
+ 0);
+ int mmap_errno = errno;
+ EXPECT_EQ(MAP_FAILED, writable)
+ << "It shouldn't be possible to re-mmap the descriptor writable.";
+ EXPECT_EQ(EACCES, mmap_errno);
+
+ // Try to re-open through /dev/fd. This is an end-run around the notion of an
+ // FD as a capability.
+ const std::string shmem_path = StringPrintf("/dev/fd/%d", handle.fd);
+ errno = 0;
+ int readable_fd = open(shmem_path.c_str(), O_RDONLY);
+ EXPECT_NE(-1, readable_fd) << strerror(errno);
+ close(readable_fd);
+
+ errno = 0;
+ int writable_fd = open(shmem_path.c_str(), O_WRONLY);
+ int open_writable_errno = errno;
+ EXPECT_EQ(-1, writable_fd);
+ EXPECT_EQ(EACCES, open_writable_errno) << strerror(open_writable_errno);
+ close(writable_fd);
+
+ // However, if we explicitly make the entry in /dev/fd writable first, the
+ // open() call successfully creates a writable file on Linux. The sandbox has
+ // to prevent opening this path. TODO(jln): Write a test that attacks this
+ // from inside the sandbox.
+ fchmod(handle.fd, S_IRUSR | S_IWUSR);
jln (very slow on Chromium) 2013/10/16 00:16:39 Please, check the return value, it's better to kee
Jeffrey Yasskin 2013/10/16 01:16:39 Done.
+
+ errno = 0;
+ writable_fd = open(shmem_path.c_str(), O_WRONLY);
+ open_writable_errno = errno;
+ // Mac appears to get this right by treating open(/dev/fd/N) as dup(N).
jln (very slow on Chromium) 2013/10/16 00:16:39 This is not correct, open(/dev/fd/N) is very diffe
Jeffrey Yasskin 2013/10/16 01:16:39 https://developer.apple.com/library/mac/documentat
jln (very slow on Chromium) 2013/10/16 17:51:15 Ohh yeah, F_DUPFD is confusing. It really has noth
+#if !OS_LINUX
jln (very slow on Chromium) 2013/10/16 00:16:39 Style: #if !defined()
Jeffrey Yasskin 2013/10/16 01:16:39 Done.
+ EXPECT_EQ(-1, writable_fd);
+ EXPECT_EQ(EACCES, open_writable_errno) << strerror(open_writable_errno);
+#endif
+ close(writable_fd);
+
+#elif OS_WIN
Jeffrey Yasskin 2013/10/15 23:03:52 Please try to think of other attacks I should be t
+ EXPECT_EQ(NULL, MapViewOfFile(handle, FILE_MAP_WRITE, 0, 0, 0))
+ << "Shouldn't be able to map memory writable.";
+
+ SharedMemoryHandle writable_handle = INVALID_HANDLE_VALUE;
+ EXPECT_EQ(0,
+ ::DuplicateHandle(GetCurrentProcess(),
+ handle,
+ GetCurrentProcess,
+ &writable_handle,
+ FILE_MAP_ALL_ACCESS,
+ false,
+ 0))
+ << "Shouldn't be able to duplicate the handle into a writable one.";
+#else
+#error Unexpected platform; write a test that tries to make 'handle' writable.
+#endif
+}
+
TEST(SharedMemoryTest, MapAt) {
ASSERT_TRUE(SysInfo::VMAllocationGranularity() >= sizeof(uint32));
const size_t kCount = SysInfo::VMAllocationGranularity();

Powered by Google App Engine
This is Rietveld 408576698