Mojo: Make sure you can't send either end of a message pipe over itself.

Previously, I had only checked that you can't write a message with a
handle to itself. I also need to check that you can't write the handle's
"peer" to the handle (which is actually worse).

R=darin@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@248594 0039d316-1c4b-4281-b951-d872f2087c98

mojo/system/core_impl_unittest.cc

@@ -440,10 +440,6 @@ TEST_F(CoreImplTest, MessagePipeBasicLocalHandlePassing1) {
   EXPECT_EQ(MOJO_RESULT_OK,
             core()->CreateMessagePipe(&h_passing[0], &h_passing[1]));
 
-  MojoHandle h_passed[2];
-  EXPECT_EQ(MOJO_RESULT_OK,
-            core()->CreateMessagePipe(&h_passed[0], &h_passed[1]));
-
   // Make sure that |h_passing[]| work properly.
   EXPECT_EQ(MOJO_RESULT_OK,
             core()->WriteMessage(h_passing[0],
@@ -463,6 +459,23 @@ TEST_F(CoreImplTest, MessagePipeBasicLocalHandlePassing1) {
   EXPECT_STREQ(kHello, buffer);
   EXPECT_EQ(0u, num_handles);
 
+  // Make sure that you can't pass either of the message pipe's handles over
+  // itself.
+  EXPECT_EQ(MOJO_RESULT_BUSY,
+            core()->WriteMessage(h_passing[0],
+                                 kHello, kHelloSize,
+                                 &h_passing[0], 1,
+                                 MOJO_WRITE_MESSAGE_FLAG_NONE));
+  EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT,
+            core()->WriteMessage(h_passing[0],
+                                 kHello, kHelloSize,
+                                 &h_passing[1], 1,
+                                 MOJO_WRITE_MESSAGE_FLAG_NONE));
+
+  MojoHandle h_passed[2];
+  EXPECT_EQ(MOJO_RESULT_OK,
+            core()->CreateMessagePipe(&h_passed[0], &h_passed[1]));
+
   // Make sure that |h_passed[]| work properly.
   EXPECT_EQ(MOJO_RESULT_OK,
             core()->WriteMessage(h_passed[0],

mojo/system/dispatcher.h

@@ -45,8 +45,9 @@ class MOJO_SYSTEM_IMPL_EXPORT Dispatcher :
   MojoResult Close();
   // |dispatchers| may be non-null if and only if there are handles to be
   // written, in which case this will be called with all the dispatchers' locks
-  // held. On success, all the dispatchers must have been moved to a closed
-  // state; on failure, they should remain in their original state.
+  // held; |this| must not be in |dispatchers|. On success, all the dispatchers
+  // must have been moved to a closed state; on failure, they should remain in
+  // their original state.
   MojoResult WriteMessage(const void* bytes,
                           uint32_t num_bytes,
                           const std::vector<Dispatcher*>* dispatchers,

mojo/system/message_pipe.cc

@@ -9,6 +9,7 @@
 #include "mojo/system/dispatcher.h"
 #include "mojo/system/local_message_pipe_endpoint.h"
 #include "mojo/system/message_in_transit.h"
+#include "mojo/system/message_pipe_dispatcher.h"
 #include "mojo/system/message_pipe_endpoint.h"
 #include "mojo/system/proxy_message_pipe_endpoint.h"
 
@@ -132,6 +133,21 @@ MojoResult MessagePipe::EnqueueMessage(
     return MOJO_RESULT_FAILED_PRECONDITION;
   }
 
+  if (dispatchers) {
+    for (size_t i = 0; i < dispatchers->size(); i++) {
+      if ((*dispatchers)[i]->GetType() == Dispatcher::kTypeMessagePipe) {
+        MessagePipeDispatcher* mp_dispatcher =
+            static_cast<MessagePipeDispatcher*>((*dispatchers)[i]);
+        if (mp_dispatcher->GetMessagePipeNoLock() == this) {
+          // The other case should have been disallowed by |CoreImpl|. (Note:
+          // |port| is the peer port of the handle given to |WriteMessage()|.)
+          DCHECK_EQ(mp_dispatcher->GetPortNoLock(), port);
+          return MOJO_RESULT_INVALID_ARGUMENT;
+        }
+      }
+    }
+  }
+
   return endpoints_[port]->EnqueueMessage(message, dispatchers);
 }