nacl_io: socketpair: Fix missing lock

UnixEventEmitter::{ReadIn,WriteOut}_Locked() call peer->UpdateStatus_Locked()
without locking "peer".  The shared FIFOs are protected from concurrent
access, but the peer's listener map isn't.  This results in a NaCl crash
under load.

Fix this by using the master's fifo_lock_ to guard accesses to either
emitter's *_Locked functions.

BUG=584997
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_nacl_sdk;tryserver.chromium.mac:mac_nacl_sdk;tryserver.chromium.win:win_nacl_sdk

Review URL: https://codereview.chromium.org/1685493002

Cr-Commit-Position: refs/heads/master@{#376391}

4 files changed, 5 insertions, 13 deletions

diff --git a/native_client_sdk/src/libraries/nacl_io/event_emitter.cc b/native_client_sdk/src/libraries/nacl_io/event_emitter.cc
index 2e620bc..c096b2e 100644
--- a/native_client_sdk/src/libraries/nacl_io/event_emitter.cc
+++ b/native_client_sdk/src/libraries/nacl_io/event_emitter.cc
@@ -22,12 +22,12 @@ EventEmitter::EventEmitter() : event_status_(0) {
 }
 
 void EventEmitter::RegisterListener(EventListener* listener, uint32_t events) {
-  AUTO_LOCK(emitter_lock_);
+  AUTO_LOCK(GetLock());
   RegisterListener_Locked(listener, events);
 }
 
 void EventEmitter::UnregisterListener(EventListener* listener) {
-  AUTO_LOCK(emitter_lock_);
+  AUTO_LOCK(GetLock());
   UnregisterListener_Locked(listener);
 }
 
diff --git a/native_client_sdk/src/libraries/nacl_io/event_emitter.h b/native_client_sdk/src/libraries/nacl_io/event_emitter.h
index ed404d5..a54fb10 100644
--- a/native_client_sdk/src/libraries/nacl_io/event_emitter.h
+++ b/native_client_sdk/src/libraries/nacl_io/event_emitter.h
@@ -43,13 +43,13 @@ class EventEmitter : public sdk_util::RefObject {
   // This returns a snapshot, to ensure the status doesn't change from
   // fetch to use, hold the lock and call GetEventStatus_Locked.
   uint32_t GetEventStatus() {
-    AUTO_LOCK(emitter_lock_);
+    AUTO_LOCK(GetLock());
     return GetEventStatus_Locked();
   }
 
   uint32_t GetEventStatus_Locked() { return event_status_; }
 
-  sdk_util::SimpleLock& GetLock() { return emitter_lock_; }
+  virtual sdk_util::SimpleLock& GetLock() { return emitter_lock_; }
 
   // Updates the specified bits in the event status, and signals any
   // listeners waiting on those bits.
diff --git a/native_client_sdk/src/libraries/nacl_io/socket/unix_event_emitter.cc b/native_client_sdk/src/libraries/nacl_io/socket/unix_event_emitter.cc
index b0a7bf3..a90039c 100644
--- a/native_client_sdk/src/libraries/nacl_io/socket/unix_event_emitter.cc
+++ b/native_client_sdk/src/libraries/nacl_io/socket/unix_event_emitter.cc
@@ -43,12 +43,10 @@ class UnixMasterEventEmitter : public UnixEventEmitter {
  protected:
   virtual FIFOInterface* in_fifo() { return in_fifo_; }
   virtual FIFOInterface* out_fifo() { return out_fifo_; }
-  virtual const sdk_util::SimpleLock& GetFifoLock() { return fifo_lock_; }
 
  private:
   FIFOInterface* in_fifo_;
   FIFOInterface* out_fifo_;
-  sdk_util::SimpleLock fifo_lock_;
   bool child_emitter_created_;
   UnixChildEventEmitter* child_emitter_;
 
@@ -62,15 +60,13 @@ class UnixChildEventEmitter : public UnixEventEmitter {
     UpdateStatus_Locked();
   }
   virtual ScopedUnixEventEmitter GetPeerEmitter() { return parent_emitter_; }
+  virtual sdk_util::SimpleLock& GetLock() { return parent_emitter_->GetLock(); }
 
  protected:
   virtual void Destroy() { parent_emitter_->child_emitter_ = NULL; }
 
   virtual FIFOInterface* in_fifo() { return parent_emitter_->out_fifo(); }
   virtual FIFOInterface* out_fifo() { return parent_emitter_->in_fifo(); }
-  virtual const sdk_util::SimpleLock& GetFifoLock() {
-    return parent_emitter_->GetFifoLock();
-  }
 
  private:
   ScopedUnixMasterEventEmitter parent_emitter_;
@@ -85,7 +81,6 @@ ScopedUnixEventEmitter UnixMasterEventEmitter::GetPeerEmitter() {
 }
 
 uint32_t UnixEventEmitter::ReadIn_Locked(char* data, uint32_t len) {
-  AUTO_LOCK(GetFifoLock());
   uint32_t count = in_fifo()->Read(data, len);
   ScopedUnixEventEmitter peer = GetPeerEmitter();
   if (peer) {
@@ -96,7 +91,6 @@ uint32_t UnixEventEmitter::ReadIn_Locked(char* data, uint32_t len) {
 }
 
 uint32_t UnixEventEmitter::WriteOut_Locked(const char* data, uint32_t len) {
-  AUTO_LOCK(GetFifoLock());
   uint32_t count = out_fifo()->Write(data, len);
   ScopedUnixEventEmitter peer = GetPeerEmitter();
   if (peer) {
diff --git a/native_client_sdk/src/libraries/nacl_io/socket/unix_event_emitter.h b/native_client_sdk/src/libraries/nacl_io/socket/unix_event_emitter.h
index f1dba0d..378402e 100644
--- a/native_client_sdk/src/libraries/nacl_io/socket/unix_event_emitter.h
+++ b/native_client_sdk/src/libraries/nacl_io/socket/unix_event_emitter.h
@@ -34,8 +34,6 @@ class UnixEventEmitter : public StreamEventEmitter {
  protected:
   UnixEventEmitter() {}
 
-  // Probably only need the master's lock.
-  virtual const sdk_util::SimpleLock& GetFifoLock() = 0;
   virtual FIFOInterface* in_fifo() = 0;
   virtual FIFOInterface* out_fifo() = 0;