New test and off-by-one fix for data persisted to disk.

base/metrics/persistent_memory_allocator.cc

 // Copyright (c) 2015 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/metrics/persistent_memory_allocator.h"
 
 #include <assert.h>
 #include <algorithm>
 
 #include "base/files/memory_mapped_file.h"
@@ skipping 145 matching lines @@
 
   // These atomics operate inter-process and so must be lock-free. The local
   // casts are to make sure it can be evaluated at compile time to a constant.
   CHECK(((SharedMetadata*)0)->freeptr.is_lock_free());
   CHECK(((SharedMetadata*)0)->flags.is_lock_free());
   CHECK(((BlockHeader*)0)->next.is_lock_free());
   CHECK(corrupt_.is_lock_free());
 
   if (shared_meta()->cookie != kGlobalCookie) {
     if (readonly) {
-      NOTREACHED();
       SetCorrupt();
       return;
     }
 
     // This block is only executed when a completely new memory segment is
     // being initialized. It's unshared and single-threaded...
     volatile BlockHeader* const first_block =
         reinterpret_cast<volatile BlockHeader*>(mem_base_ +
                                                 sizeof(SharedMetadata));
     if (shared_meta()->cookie != 0 ||
@@ skipping 31 matching lines @@
 
     // Allocate space for the name so other processes can learn it.
     if (!name.empty()) {
       const size_t name_length = name.length() + 1;
       shared_meta()->name = Allocate(name_length, 0);
       char* name_cstr = GetAsObject<char>(shared_meta()->name, 0);
       if (name_cstr)
         strcpy(name_cstr, name.c_str());
     }
   } else {
-    if (readonly) {
-      // For read-only access, validate reasonable ctor parameters.
-      DCHECK_GE(mem_size_, shared_meta()->freeptr.load());
-    } else {
+    if (!readonly) {
       // The allocator is attaching to a previously initialized segment of
       // memory. Make sure the embedded data matches what has been passed.
       if (shared_meta()->size != mem_size_ ||
           shared_meta()->page_size != mem_page_) {
         NOTREACHED();
         SetCorrupt();
       }
     }
   }
 }
@@ skipping 43 matching lines @@
   return std::min(shared_meta()->freeptr.load(), mem_size_);
 }
 
 size_t PersistentMemoryAllocator::GetAllocSize(Reference ref) const {
   const volatile BlockHeader* const block = GetBlock(ref, 0, 0, false, false);
   if (!block)
     return 0;
   uint32_t size = block->size;
   // Header was verified by GetBlock() but a malicious actor could change
   // the value between there and here. Check it again.
-  if (size <= sizeof(BlockHeader) || ref + size >= mem_size_) {
+  if (size <= sizeof(BlockHeader) || ref + size > mem_size_) {
     SetCorrupt();
     return 0;
   }
   return size - sizeof(BlockHeader);
 }
 
 uint32_t PersistentMemoryAllocator::GetType(Reference ref) const {
   const volatile BlockHeader* const block = GetBlock(ref, 0, 0, false, false);
   if (!block)
     return 0;
@@ skipping 373 matching lines @@
 FilePersistentMemoryAllocator::~FilePersistentMemoryAllocator() {
 }
 
 // static
 bool FilePersistentMemoryAllocator::IsFileAcceptable(
     const MemoryMappedFile& file) {
   return IsMemoryAcceptable(file.data(), file.length(), 0, true);
 }
 
 }  // namespace base