Bug 1732343 - Part 6: Use attached handles for Windows TransportDescriptor serialization, r=handyman

Differential Revision: https://phabricator.services.mozilla.com/D126568

ipc/glue/AutoTransportDescriptor.cpp

@@ -33,9 +33,9 @@ AutoTransportDescriptor::~AutoTransportDescriptor() {
 }
 
 Result<std::pair<AutoTransportDescriptor, AutoTransportDescriptor>, nsresult>
-AutoTransportDescriptor::Create(int32_t aProcIdOne) {
+AutoTransportDescriptor::Create() {
   TransportDescriptor one, two;
-  MOZ_TRY(CreateTransport(aProcIdOne, &one, &two));
+  MOZ_TRY(CreateTransport(&one, &two));
   return std::pair{AutoTransportDescriptor(one), AutoTransportDescriptor(two)};
 }
 

ipc/glue/AutoTransportDescriptor.h

@@ -26,7 +26,7 @@ class AutoTransportDescriptor final {
 
   static Result<std::pair<AutoTransportDescriptor, AutoTransportDescriptor>,
                 nsresult>
-  Create(int32_t aProcIdOne);
+  Create();
 
   AutoTransportDescriptor Duplicate() const;
 

ipc/glue/NodeController.cpp

@@ -651,7 +651,7 @@ void NodeController::OnRequestIntroduction(const NodeName& aFromNode,
   }
 
   RefPtr<NodeChannel> peerB = GetNodeChannel(aName);
-  auto result = AutoTransportDescriptor::Create(peerA->OtherPid());
+  auto result = AutoTransportDescriptor::Create();
   if (!peerB || result.isErr()) {
     NODECONTROLLER_WARNING(
         "Rejecting introduction request from '%s' for unknown peer '%s'",

ipc/glue/Transport.h

@@ -22,8 +22,7 @@ namespace ipc {
 
 typedef IPC::Channel Transport;
 
-nsresult CreateTransport(base::ProcessId aProcIdOne, TransportDescriptor* aOne,
-                         TransportDescriptor* aTwo);
+nsresult CreateTransport(TransportDescriptor* aOne, TransportDescriptor* aTwo);
 
 UniquePtr<Transport> OpenDescriptor(const TransportDescriptor& aTd,
                                     Transport::Mode aMode);

ipc/glue/Transport_posix.cpp

@@ -20,8 +20,7 @@ using base::ProcessHandle;
 namespace mozilla {
 namespace ipc {
 
-nsresult CreateTransport(base::ProcessId aProcIdOne, TransportDescriptor* aOne,
-                         TransportDescriptor* aTwo) {
+nsresult CreateTransport(TransportDescriptor* aOne, TransportDescriptor* aTwo) {
   auto id = IPC::Channel::GenerateVerifiedChannelID();
   // Use MODE_SERVER to force creation of the socketpair
   Transport t(id, Transport::MODE_SERVER, nullptr);

ipc/glue/Transport_win.cpp

@@ -8,71 +8,44 @@
 
 #include "mozilla/ipc/Transport.h"
 #include "mozilla/ipc/ProtocolUtils.h"
+#include <windows.h>
 
 using base::ProcessHandle;
 
 namespace mozilla {
 namespace ipc {
 
-nsresult CreateTransport(base::ProcessId aProcIdOne, TransportDescriptor* aOne,
-                         TransportDescriptor* aTwo) {
+nsresult CreateTransport(TransportDescriptor* aOne, TransportDescriptor* aTwo) {
   auto id = IPC::Channel::GenerateVerifiedChannelID();
-  // Use MODE_SERVER to force creation of the pipe
-  Transport t(id, Transport::MODE_SERVER, nullptr);
-  HANDLE serverPipe = t.GetServerPipeHandle();
-  if (!serverPipe) {
-    return NS_ERROR_TRANSPORT_INIT;
-  }
 
+  // Use MODE_SERVER to force creation of the pipe
   // NB: we create the server pipe immediately, instead of just
   // grabbing an ID, on purpose.  In the current setup, the client
   // needs to connect to an existing server pipe, so to prevent race
   // conditions, we create the server side here. When we send the pipe
   // to the server, we DuplicateHandle it to the server process to give it
   // access.
-  HANDLE serverDup;
-  DWORD access = 0;
-  DWORD options = DUPLICATE_SAME_ACCESS;
-  if (!DuplicateHandle(serverPipe, base::GetCurrentProcId(), &serverDup, access,
-                       options)) {
+  Transport t(id, Transport::MODE_SERVER, nullptr);
+  HANDLE serverPipe = t.GetServerPipeHandle();
+  if (!serverPipe) {
+    return NS_ERROR_TRANSPORT_INIT;
+  }
+
+  // Make a copy of the handle owned by the `Transport` which will be
+  // transferred to the actual server process.
+  if (!::DuplicateHandle(GetCurrentProcess(), serverPipe, GetCurrentProcess(),
+                         &aOne->mServerPipeHandle, 0, false,
+                         DUPLICATE_SAME_ACCESS)) {
     return NS_ERROR_DUPLICATE_HANDLE;
   }
 
   aOne->mPipeName = aTwo->mPipeName = id;
-  aOne->mServerPipeHandle = serverDup;
-  aOne->mDestinationProcessId = aProcIdOne;
   aTwo->mServerPipeHandle = INVALID_HANDLE_VALUE;
-  aTwo->mDestinationProcessId = 0;
   return NS_OK;
 }
 
-HANDLE
-TransferHandleToProcess(HANDLE source, base::ProcessId pid) {
-  // At this point we're sending the handle to another process.
-
-  if (source == INVALID_HANDLE_VALUE) {
-    return source;
-  }
-  HANDLE handleDup;
-  DWORD access = 0;
-  DWORD options = DUPLICATE_SAME_ACCESS;
-  bool ok = DuplicateHandle(source, pid, &handleDup, access, options);
-  if (!ok) {
-    return nullptr;
-  }
-
-  // Now close our own copy of the handle (we're supposed to be transferring,
-  // not copying).
-  CloseHandle(source);
-
-  return handleDup;
-}
-
 UniquePtr<Transport> OpenDescriptor(const TransportDescriptor& aTd,
                                     Transport::Mode aMode) {
-  if (aTd.mServerPipeHandle != INVALID_HANDLE_VALUE) {
-    MOZ_RELEASE_ASSERT(aTd.mDestinationProcessId == base::GetCurrentProcId());
-  }
   return MakeUnique<Transport>(aTd.mPipeName, aTd.mServerPipeHandle, aMode,
                                nullptr);
 }
@@ -85,10 +58,9 @@ TransportDescriptor DuplicateDescriptor(const TransportDescriptor& aTd) {
   }
 
   HANDLE serverDup;
-  DWORD access = 0;
-  DWORD options = DUPLICATE_SAME_ACCESS;
-  bool ok = DuplicateHandle(aTd.mServerPipeHandle, base::GetCurrentProcId(),
-                            &serverDup, access, options);
+  bool ok = ::DuplicateHandle(GetCurrentProcess(), aTd.mServerPipeHandle,
+                              GetCurrentProcess(), &serverDup, 0, false,
+                              DUPLICATE_SAME_ACCESS);
   if (!ok) {
     AnnotateSystemError();
   }

ipc/glue/Transport_win.h

@@ -13,6 +13,7 @@
 #include "ipc/IPCMessageUtils.h"
 #include "nsWindowsHelpers.h"
 #include "nsXULAppAPI.h"
+#include "mozilla/UniquePtrExtensions.h"
 
 namespace mozilla {
 namespace ipc {
@@ -23,9 +24,6 @@ struct TransportDescriptor {
   base::ProcessId mDestinationProcessId;
 };
 
-HANDLE
-TransferHandleToProcess(HANDLE source, base::ProcessId pid);
-
 }  // namespace ipc
 }  // namespace mozilla
 
@@ -35,71 +33,25 @@ template <>
 struct ParamTraits<mozilla::ipc::TransportDescriptor> {
   typedef mozilla::ipc::TransportDescriptor paramType;
   static void Write(Message* aMsg, const paramType& aParam) {
-    HANDLE pipe = mozilla::ipc::TransferHandleToProcess(
-        aParam.mServerPipeHandle, aParam.mDestinationProcessId);
-    DWORD duplicateFromProcessId = 0;
-    if (!pipe) {
-      if (XRE_IsParentProcess()) {
-        // If we are the parent and failed to transfer then there is no hope,
-        // just close the handle.
-        ::CloseHandle(aParam.mServerPipeHandle);
-      } else {
-        // We are probably sending to parent so it should be able to duplicate.
-        pipe = aParam.mServerPipeHandle;
-        duplicateFromProcessId = ::GetCurrentProcessId();
-      }
-    }
-
     WriteParam(aMsg, aParam.mPipeName);
-    WriteParam(aMsg, pipe);
-    WriteParam(aMsg, duplicateFromProcessId);
+    WriteParam(aMsg, mozilla::UniqueFileHandle(aParam.mServerPipeHandle));
     WriteParam(aMsg, aParam.mDestinationProcessId);
   }
   static bool Read(const Message* aMsg, PickleIterator* aIter,
                    paramType* aResult) {
-    DWORD duplicateFromProcessId;
+    mozilla::UniqueFileHandle serverPipeHandle;
     bool r = (ReadParam(aMsg, aIter, &aResult->mPipeName) &&
-              ReadParam(aMsg, aIter, &aResult->mServerPipeHandle) &&
-              ReadParam(aMsg, aIter, &duplicateFromProcessId) &&
+              ReadParam(aMsg, aIter, &serverPipeHandle) &&
               ReadParam(aMsg, aIter, &aResult->mDestinationProcessId));
     if (!r) {
       return r;
     }
 
-    MOZ_RELEASE_ASSERT(
-        aResult->mServerPipeHandle,
-        "Main process failed to duplicate pipe handle to child.");
-
-    // If this is a not the "server" side descriptor, we have finished.
-    if (aResult->mServerPipeHandle == INVALID_HANDLE_VALUE) {
-      return true;
-    }
-
-    MOZ_RELEASE_ASSERT(aResult->mDestinationProcessId ==
-                       base::GetCurrentProcId());
-
-    // If the pipe has already been duplicated to us, we have finished.
-    if (!duplicateFromProcessId) {
-      return true;
-    }
-
-    // Otherwise duplicate the handle to us.
-    nsAutoHandle sourceProcess(
-        ::OpenProcess(PROCESS_DUP_HANDLE, FALSE, duplicateFromProcessId));
-    if (!sourceProcess) {
-      return false;
-    }
-
-    HANDLE ourHandle;
-    BOOL duped = ::DuplicateHandle(
-        sourceProcess, aResult->mServerPipeHandle, ::GetCurrentProcess(),
-        &ourHandle, 0, FALSE, DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS);
-    if (!duped) {
+    if (serverPipeHandle) {
+      aResult->mServerPipeHandle = serverPipeHandle.release();
+    } else {
       aResult->mServerPipeHandle = INVALID_HANDLE_VALUE;
-      return false;
     }
-
-    aResult->mServerPipeHandle = ourHandle;
     return true;
   }
 };