Mojo: Remove UAF workaround

Allocation padding was added to the global port map to avoid
being stomped on by a UAF somewhere on Chrome for Android.

The culprit has likely been found and a fix is imminent. See
bug 673417 for details.

BUG=740044,725605
TBR=jcivelli@chromium.org

Change-Id: I934c0e7a6a84a7eb21a74d122f446e14275babbf
Reviewed-on: https://chromium-review.googlesource.com/688492Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505051}

mojo/edk/system/ports/node.cc

@@ -114,10 +114,8 @@ bool Node::CanShutdownCleanly(ShutdownPolicy policy) {
 #if DCHECK_IS_ON()
     for (auto& entry : ports_) {
       DVLOG(2) << "Port " << entry.first << " referencing node "
-               << entry.second.port->peer_node_name
-               << " is blocking shutdown of "
-               << "node " << name_ << " (state=" << entry.second.port->state
-               << ")";
+               << entry.second->peer_node_name << " is blocking shutdown of "
+               << "node " << name_ << " (state=" << entry.second->state << ")";
     }
 #endif
     return ports_.empty();
@@ -130,7 +128,7 @@ bool Node::CanShutdownCleanly(ShutdownPolicy policy) {
   // need to be blazingly fast.
   bool can_shutdown = true;
   for (auto& entry : ports_) {
-    PortRef port_ref(entry.first, entry.second.port);
+    PortRef port_ref(entry.first, entry.second);
     SinglePortLocker locker(&port_ref);
     auto* port = locker.port();
     if (port->peer_node_name != name_ && port->state != Port::kReceiving) {
@@ -161,7 +159,7 @@ int Node::GetPort(const PortName& port_name, PortRef* port_ref) {
   base::subtle::MemoryBarrier();
 #endif
 
-  *port_ref = PortRef(port_name, iter->second.port);
+  *port_ref = PortRef(port_name, iter->second);
   return OK;
 }
 
@@ -783,7 +781,7 @@ int Node::OnMergePort(std::unique_ptr<MergePortEvent> event) {
 int Node::AddPortWithName(const PortName& port_name, scoped_refptr<Port> port) {
   PortLocker::AssertNoPortsLockedOnCurrentThread();
   base::AutoLock lock(ports_lock_);
-  if (!ports_.emplace(port_name, PortTableEntry(std::move(port))).second)
+  if (!ports_.emplace(port_name, std::move(port)).second)
     return OOPS(ERROR_PORT_EXISTS);  // Suggests a bad UUID generator.
   DVLOG(2) << "Created port " << port_name << "@" << name_;
   return OK;
@@ -797,7 +795,7 @@ void Node::ErasePort(const PortName& port_name) {
     auto it = ports_.find(port_name);
     if (it == ports_.end())
       return;
-    port = std::move(it->second.port);
+    port = std::move(it->second);
     ports_.erase(it);
   }
   // NOTE: We are careful not to release the port's messages while holding any
@@ -1293,7 +1291,7 @@ void Node::DestroyAllPortsWithPeer(const NodeName& node_name,
     base::AutoLock ports_lock(ports_lock_);
 
     for (auto iter = ports_.begin(); iter != ports_.end(); ++iter) {
-      PortRef port_ref(iter->first, iter->second.port);
+      PortRef port_ref(iter->first, iter->second);
       {
         SinglePortLocker locker(&port_ref);
         auto* port = locker.port();
@@ -1323,7 +1321,7 @@ void Node::DestroyAllPortsWithPeer(const NodeName& node_name,
           if (port->state != Port::kReceiving) {
             dead_proxies_to_broadcast.push_back(iter->first);
             std::vector<std::unique_ptr<UserMessageEvent>> messages;
-            iter->second.port->message_queue.TakeAllMessages(&messages);
+            iter->second->message_queue.TakeAllMessages(&messages);
             for (auto& message : messages)
               undelivered_messages.emplace_back(std::move(message));
           }
@@ -1378,23 +1376,6 @@ void Node::DelegateHolder::EnsureSafeDelegateAccess() const {
 }
 #endif
 
-Node::PortTableEntry::PortTableEntry(scoped_refptr<Port> port)
-    : port(std::move(port)) {}
-
-Node::PortTableEntry::PortTableEntry(PortTableEntry&& other) = default;
-
-Node::PortTableEntry::~PortTableEntry() {
-  // We don't really anticipate hitting this CHECK given that we should be
-  // effectively working around memory corruption. Still it's a potentially
-  // useful signal if anything continues to go wrong, and it forces the value to
-  // be used.
-  CHECK_EQ(0x1827364554637281ULL, sentinel_value_);
-  sentinel_value_ = 0x8127364554637281ULL;
-}
-
-Node::PortTableEntry& Node::PortTableEntry::operator=(PortTableEntry&& other) =
-    default;
-
 }  // namespace ports
 }  // namespace edk
 }  // namespace mojo

mojo/edk/system/ports/node.h

@@ -227,24 +227,6 @@ class Node {
   const NodeName name_;
   const DelegateHolder delegate_;
 
-  // A wrapper structure for Port table entries. https://crbug.com/725605 likely
-  // indicates memory corruption stomping on the Port table, and there is no
-  // clue as to where the corruption is coming from. Changing the size of the
-  // map entry allocations should effectively work around the bug, as it did for
-  // https://crbug.com/740044.
-  struct PortTableEntry {
-    explicit PortTableEntry(scoped_refptr<Port> port);
-    PortTableEntry(const PortTableEntry&) = delete;
-    PortTableEntry(PortTableEntry&& other);
-    ~PortTableEntry();
-
-    PortTableEntry& operator=(const PortTableEntry&) = delete;
-    PortTableEntry& operator=(PortTableEntry&& other);
-
-    uint64_t sentinel_value_ = 0x1827364554637281ULL;
-    scoped_refptr<Port> port;
-  };
-
   // Guards |ports_|. This must never be acquired while an individual port's
   // lock is held on the same thread. Conversely, individual port locks may be
   // acquired while this one is held.
@@ -253,7 +235,7 @@ class Node {
   // destruction, it is also important to ensure that such events are never
   // destroyed while this (or any individual Port) lock is held.
   base::Lock ports_lock_;
-  std::unordered_map<PortName, PortTableEntry> ports_;
+  std::unordered_map<PortName, scoped_refptr<Port>> ports_;
 
   DISALLOW_COPY_AND_ASSIGN(Node);
 };