Create "persistent memory allocator" for persisting and sharing objects.

base/memory/shared_memory_allocator.h

+// 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.
+
+#ifndef BASE_MEMORY_SHARED_MEMORY_ALLOCATOR_H_
+#define BASE_MEMORY_SHARED_MEMORY_ALLOCATOR_H_
+
+#include <stdint.h>
+
+#include "base/atomicops.h"
+#include "base/base_export.h"
+#include "base/macros.h"
+
+namespace base {
+
+// Simple allocator for pieces of a memory block that may be shared across
+// multiple processes.
+//
+// This class provides for thread-secure (i.e. safe against other threads

[Alexander Potapenko, 2015/11/09 18:03:46]

Has this design been reviewed by the security team already?
I'd say corruption detection is nice, but that's a best-effort security
mechanism and should not be relied on much (I'm not a real security engineer
though).

[bcwhite, 2015/11/09 19:21:16]

Yes, from the security team Justin Schuh gave some initial "moderately positive"
comments and Matthew Dempsky from has been over the design.  JF Bastien also
wants to take a closer look when he's back from vacation.

+// or processes that may be compromised and thus have malicious intent)
+// allocation of memory within a designated block and also a mechanism by
+// which other threads can learn of the allocations with any additional
+// shared information.
+//
+// There is (currently) no way to release an allocated block of data because
+// doing so would risk invalidating pointers held by other processes and
+// greatly complicate the allocation algorithm.
+//
+// Construction of this object can accept new, clean (i.e. zeroed) memory
+// or previously initialized memory. In the first case, construction must
+// be allowed to complete before letting other allocators attach to the same
+// segment. In other words, don't share the segment until at least one
+// allocator has been attached to it.
+//
+// It should be noted that memory doesn't need to actually have zeros written
+// throughout; it just needs to read as zero until something diffferent is
+// written to a location. This is an important distinction as it supports the
+// use-case of non-pinned memory, such as from a demand-allocated region by
+// the OS or a memory-mapped file that auto-grows from a starting size of zero.
+class BASE_EXPORT SharedMemoryAllocator {
+ public:
+  typedef int32_t Reference;
+
+  // Internal state information when iterating over memory allocations.
+  struct Iterator {
+    Reference last;
+    uint32_t niter;
+  };
+
+  // Returned information about the internal state of the heap.
+  struct MemoryInfo {
+    size_t total;
+    size_t free;
+  };
+
+  enum : uint32_t {
+    kTypeIdAny = 0  // Match any type-id inside GetAsObject().

[chrisha, 2015/11/09 16:53:51]

Also worth having a int32_t kReferenceInvalid = 0?

[bcwhite, 2015/11/09 18:02:05]

I don't think it's necessary.  Like with pointers, the check for validity inside
Chrome code is always "if (myref) { ... }"

+  };
+
+  // The allocator operates on any arbitrary block of memory. Creation and
+  // sharing of that block with another process is the responsibility of the
+  // caller. The allocator needs to know only the block's |base| address, the
+  // total |size| of the block, and any internal |page| size (zero if not
+  // paged) across which allocations should not span.
+  //
+  // SharedMemoryAllocator does NOT take ownership of this memory block. The
+  // caller must manage it and ensure it stays available throughout the lifetime
+  // of this object.
+  //
+  // Memory segments for sharing must have had an allocator attached to them
+  // before actually being shared. If the memory segment was just created, it
+  // should be zeroed. If it was an existing segment, the values here will
+  // be compared to copies stored in the shared segment as a guard against
+  // corruption.
+  SharedMemoryAllocator(void* base, size_t size, size_t page_size);
+  ~SharedMemoryAllocator();
+
+  // Get an object referenced by an |ref|. For safety reasons, the |type_id|

[chrisha, 2015/11/09 16:53:51]

by a* |ref|

[bcwhite, 2015/11/09 18:02:05]

Doh!  I *knew* I was going to miss something in the rename from object->ref.

+  // code and size-of(|T|) are compared to ensure the reference is valid
+  // and cannot return an object outside of the memory segment. A |type_id| of
+  // zero will match any though the size is still checked. NULL is returned
+  // if any problem is detected, such as corrupted storage or incorrect
+  // parameters. Callers MUST check that the returned value is not-null EVERY
+  // TIME before accessing it or risk crashing!  Once dereferenced, the pointer
+  // is safe to reuse forever.
+  //
+  // NOTE: Though this method will guarantee that an object of the specified
+  // type can be accessed without going outside the bounds of the memory
+  // segment, it makes not guarantees of the validity of the data within the

[chrisha, 2015/11/09 16:53:51]

no*

[bcwhite, 2015/11/09 18:02:05]

Done.

+  // object itself. If it is expected that the contents of the segment could
+  // be compromised with malicious intent, the object must be hardened as well.
+  template <typename T>
+  T* GetAsObject(Reference ref, uint32_t type_id) {

[chrisha, 2015/11/09 16:53:51]

This function also wants a counterpart to convert a typed pointer to an offset,
that also checks validity, size, etc.

(Especially if you want users to be able to bake-in 'reference' based pointers.
I'd keep the exact format of Reference as completely opaque and expect users to
convert back and forth via the API. This also allows you to change how its
represented in the future.)

[bcwhite, 2015/11/09 18:02:05]

Possibly.  I've left it out because I haven't come across cause to use it and
I've generally been asked to leave out code "for future use".

+    return static_cast<T*>(GetBlockData(ref, type_id, sizeof(T)));
+  }
+
+  // Get the number of bytes allocated to a block. This is useful when storing
+  // arrays in order to validate the ending boundary.  The returned value will
+  // include any padding added to achieve the required alignment and so could
+  // be larger than given in the original Allocate() request.
+  size_t GetAllocSize(Reference ref);
+
+  // Reserve space in the memory segment of the desired |size| and |type_id|.
+  // A return value of zero indicates the allocation failed, otherwise the
+  // returned reference can be used by any process to get a real pointer via
+  // the GetAsObject() call.
+  int32_t Allocate(size_t size, uint32_t type_id);
+
+  // Allocated objects can be added to an internal list that can then be
+  // iterated over by other processes. If an allocated object can be found
+  // another way, such as by having its reference within a different object
+  // that will be made iterable, then this call is not necessary. This always
+  // succeeds unless corruption is detected; check IsCorrupted() to find out.
+  void MakeIterable(Reference ref);

[chrisha, 2015/11/09 16:53:51]

Can't this return true on success, false on failure? Avoids having to call
'IsCorrupted' after every call to MakeIterable.

[bcwhite, 2015/11/09 18:02:05]

It's never supposed to fail so I don't think there as a need to check.  If there
is corruption then it's likely there is nothing the caller can do.  A return
flag would indicate that the caller should check the value and act but that
would be code never executed except in the presence of a serious failure.

+
+  // Get the information about the amount of free space in the allocator. The
+  // amount of free space should be treated as approximate due to extras from
+  // alignment and metadata. Concurrent allocations from other threads will
+  // also make the true amount less than what is reported. It will never
+  // return _less_ than could actually be allocated.

[chrisha, 2015/11/09 16:53:51]

This last sentence is at odds with the sentence before it. Either suffix it with
" at the exact moment of the query.", or simply remove it altogether?

[bcwhite, 2015/11/09 18:02:05]

Done.

+  void GetMemoryInfo(MemoryInfo* meminfo);
+
+  // Iterating uses a |state| structure (initialized by CreateIterator) and
+  // returns both the reference reference to the object as well as the |type_id|

[chrisha, 2015/11/09 16:53:51]

reference reference

[bcwhite, 2015/11/09 18:02:05]

Done.

+  // of that object. A zero return value indicates there are currently no more
+  // objects to be found but future attempts can be made without having to
+  // reset the iterator to "first".
+  void CreateIterator(Iterator* state);
+  int32_t GetNextIterable(Iterator* state, uint32_t* type_id);
+
+  // If there is some indication that the shared memory has become corrupted,
+  // calling this will attempt to prevent further damage by indicating to
+  // all processes that something is not as expected.
+  void SetCorrupt();
+
+  // This can be called to determine if corruption has been detected in the
+  // shared segment, possibly my a malicious actor. Once detected, future
+  // allocations will fail and iteration may not locate all objects.
+  bool IsCorrupt();
+
+  // Flag set if an allocation has failed because memory was full.
+  bool IsFull();
+
+ private:
+  struct SharedMetadata;
+  struct BlockHeader;
+
+  BlockHeader* GetBlock(Reference ref, uint32_t type_id, int32_t size,
+                        bool queue_ok, bool free_ok);
+  void* GetBlockData(Reference ref, uint32_t type_id, int32_t size);
+
+  SharedMetadata* shared_meta_; // Pointer to start of memory segment.
+  char* mem_base_;              // Same. (char because sizeof guaranteed 1)

[chrisha, 2015/11/09 16:53:51]

This can be replaced with a pair of member functions:

char* mem_base() { return reinterpret_cast<char*>(shared_meta_); }
const char* mem_base() const { return reinterpret_cast<const
char*>(shared_meta_); }

[bcwhite, 2015/11/09 18:02:05]

Done.

+  int32_t mem_size_;            // Size of entire memory segment.
+  int32_t mem_page_;            // Page size allocations shouldn't cross.
+  subtle::Atomic32 corrupted_;  // TODO(bcwhite): Use std::atomic<char> when ok.

[chrisha, 2015/11/09 16:53:51]

This should be called flags_, no? As it also stores a 'full' bit? I'd also nuke
the <char> comment, as you have an equivalent one in the .cc file.

[bcwhite, 2015/11/09 18:02:05]

It's only a single flag and has either a 0 or 1 value.  There has to be a
"local" copy that can't be reset by an external process.

+
+  DISALLOW_COPY_AND_ASSIGN(SharedMemoryAllocator);
+};
+
+}  // namespace base
+
+#endif  // BASE_MEMORY_SHARED_MEMORY_ALLOCATOR_H_