Support read/write memory-mapped files.

base/files/memory_mapped_file_win.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/files/memory_mapped_file.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <limits>
 
 #include "base/files/file_path.h"
 #include "base/strings/string16.h"
 #include "base/threading/thread_restrictions.h"
 
 namespace base {
 
 MemoryMappedFile::MemoryMappedFile() : data_(NULL), length_(0), image_(false) {
 }
 
 bool MemoryMappedFile::InitializeAsImageSection(const FilePath& file_name) {
   image_ = true;
   return Initialize(file_name);
 }
 
 bool MemoryMappedFile::MapFileRegionToMemory(
-    const MemoryMappedFile::Region& region) {
+    const MemoryMappedFile::Region& region,
+    Access access) {
   ThreadRestrictions::AssertIOAllowed();
 
   if (!file_.IsValid())
     return false;
 
-  int flags = image_ ? SEC_IMAGE | PAGE_READONLY : PAGE_READONLY;
+  int flags = image_ ? SEC_IMAGE : 0;

[forshaw, 2016/05/09 21:23:19]

While we're changing this I'd highly recommend we remove support for image
sections. As far as I can tell it's not used at present, is a Windows specific
addition and leads to security issues like
https://bugs.chromium.org/p/chromium/issues/detail?id=564238. 

[bcwhite, 2016/05/10 00:44:41]

Hmmm....  How about I work towards that in a different CL so as to not hold up
this one unnecessarily?

[forshaw, 2016/05/10 07:04:30]

Well we could do it in another CL, sure. I'd argue that especially in light of
the changes being made the support for image sections doesn't make any sense.
You can't have a "writable" image section, at the least passing PAGE_READONLY or
PAGE_READWRITE with SEC_IMAGE makes no difference to what actually gets mapped.
If I was being pedantic you could argue that the code should have some sort of
check (even a DCHECK) that ensures image section and writable are not being used
at the same time, but if we do that change it might just be simpler and reduce
code to just remove support (assuming no one else is using it, after all there's
not even a unit test for the image section code).

[bcwhite, 2016/05/10 21:00:54]

Since it's not actually in-use anywhere, I guess there's no delay by removing it
now.  Done.

+  uint32_t size_low = 0;
+  uint32_t size_high = 0;
+  switch (access) {
+    case READ_ONLY:
+      flags |= PAGE_READONLY;
+      break;
+    case READ_WRITE:
+      flags |= PAGE_READWRITE;
+      break;
+    case READ_WRITE_EXTEND:
+      flags |= PAGE_READWRITE;
+      size_high = static_cast<uint32_t>(region.size >> 32);
+      size_low = static_cast<uint32_t>(region.size & 0xFFFFFFFF);
+      break;
+  }
 
-  file_mapping_.Set(::CreateFileMapping(file_.GetPlatformFile(), NULL,
-                                        flags, 0, 0, NULL));
+  file_mapping_.Set(::CreateFileMapping(file_.GetPlatformFile(), NULL, flags,
+                                        size_high, size_low, NULL));

[erikchen, 2016/05/10 02:21:51]

A question, mostly for forshaw:

Should we be removing the access control permissions, and also giving the
FileMapping a name, as in https://codereview.chromium.org/1677163003

[forshaw, 2016/05/10 07:04:30]

I don't believe so, at least as far as I can tell there's no way of sharing a
memory mapped file, it's only for the purposes of mapping a file into the
current process. Certainly you could share the File object to another process
but the transport of PlatformFile instances should already be handled. Once at
the other side the section object would be created anew. We do hold a reference
to the file_mapping handle, but there's no method to extract it for transport.
This code also doesn't need to maintain that reference, it would be possible to
close the handle after calling MapViewOfFile and nothing would go wrong.

   if (!file_mapping_.IsValid())
     return false;
 
   LARGE_INTEGER map_start = {};
   SIZE_T map_size = 0;
   int32_t data_offset = 0;
 
   if (region == MemoryMappedFile::Region::kWholeFile) {
+    DCHECK_NE(READ_WRITE_EXTEND, access);
     int64_t file_len = file_.GetLength();
     if (file_len <= 0 || file_len > std::numeric_limits<int32_t>::max())
       return false;
     length_ = static_cast<size_t>(file_len);
   } else {
     // The region can be arbitrarily aligned. MapViewOfFile, instead, requires
     // that the start address is aligned to the VM granularity (which is
     // typically larger than a page size, for instance 32k).
     // Also, conversely to POSIX's mmap, the |map_size| doesn't have to be
     // aligned and must be less than or equal the mapped file size.
@@ skipping 10 matching lines @@
         static_cast<uint64_t>(size) > std::numeric_limits<SIZE_T>::max()) {
       DLOG(ERROR) << "Region bounds are not valid for MapViewOfFile";
       return false;
     }
     map_start.QuadPart = aligned_start;
     map_size = static_cast<SIZE_T>(size);
     length_ = static_cast<size_t>(region.size);
   }
 
   data_ = static_cast<uint8_t*>(
-      ::MapViewOfFile(file_mapping_.Get(), FILE_MAP_READ, map_start.HighPart,
-                      map_start.LowPart, map_size));
+      ::MapViewOfFile(file_mapping_.Get(),
+                      (flags & PAGE_READONLY) ? FILE_MAP_READ : FILE_MAP_WRITE,
+                      map_start.HighPart, map_start.LowPart, map_size));
   if (data_ == NULL)
     return false;
   data_ += data_offset;
   return true;
 }
 
 void MemoryMappedFile::CloseHandles() {
   if (data_)
     ::UnmapViewOfFile(data_);
   if (file_mapping_.IsValid())
     file_mapping_.Close();
   if (file_.IsValid())
     file_.Close();
 
   data_ = NULL;
   length_ = 0;
 }
 
 }  // namespace base