Bug 831307 - Allow invalid file handles to not crash child processes. r=cjones, a=blocking-tef+
authorBen Turner <bent.mozilla@gmail.com>
Thu, 24 Jan 2013 17:10:39 -0800
changeset 118323 8a27c1e402b74fe20e7b4c65bc3925224c7e8d4c
parent 118322 4c2f07276d9e498e177f98838d10fe820cd5277a
child 118324 fb2bb1320ac72692bb177472afbbb9f1ef2916b5
push id351
push userbturner@mozilla.com
push dateFri, 25 Jan 2013 01:14:48 +0000
reviewerscjones, blocking-tef
bugs831307
milestone18.0
Bug 831307 - Allow invalid file handles to not crash child processes. r=cjones, a=blocking-tef+
dom/plugins/ipc/PluginModuleParent.cpp
ipc/glue/FileDescriptor.cpp
ipc/ipdl/ipdl/lower.py
netwerk/ipc/RemoteOpenFileChild.cpp
--- a/dom/plugins/ipc/PluginModuleParent.cpp
+++ b/dom/plugins/ipc/PluginModuleParent.cpp
@@ -774,17 +774,19 @@ bool
 PluginModuleParent::RecvBackUpXResources(const FileDescriptor& aXSocketFd)
 {
 #ifndef MOZ_X11
     NS_RUNTIMEABORT("This message only makes sense on X11 platforms");
 #else
     NS_ABORT_IF_FALSE(0 > mPluginXSocketFdDup.get(),
                       "Already backed up X resources??");
     mPluginXSocketFdDup.forget();
-    mPluginXSocketFdDup.reset(aXSocketFd.PlatformHandle());
+    if (aXSocketFd.IsValid()) {
+      mPluginXSocketFdDup.reset(aXSocketFd.PlatformHandle());
+    }
 #endif
     return true;
 }
 
 void
 PluginModuleParent::NPP_URLRedirectNotify(NPP instance, const char* url,
                                           int32_t status, void* notifyData)
 {
--- a/ipc/glue/FileDescriptor.cpp
+++ b/ipc/glue/FileDescriptor.cpp
@@ -50,17 +50,17 @@ FileDescriptor::DuplicateInCurrentProces
     if (DuplicateHandle(GetCurrentProcess(), aHandle, GetCurrentProcess(),
                         &newHandle, 0, FALSE, DUPLICATE_SAME_ACCESS)) {
 #else // XP_WIN
     if ((newHandle = dup(aHandle)) != INVALID_HANDLE) {
 #endif
       mHandle = newHandle;
       return;
     }
-    NS_WARNING("Failed to duplicate file descriptor!");
+    NS_WARNING("Failed to duplicate file handle for current process!");
   }
 
   mHandle = INVALID_HANDLE;
 }
 
 void
 FileDescriptor::CloseCurrentProcessHandle()
 {
@@ -70,17 +70,17 @@ FileDescriptor::CloseCurrentProcessHandl
   // Don't actually close handles created by another process.
   if (mHandleCreatedByOtherProcess) {
     return;
   }
 
   if (IsValid()) {
 #ifdef XP_WIN
     if (!CloseHandle(mHandle)) {
-      NS_WARNING("Failed to close file handle!");
+      NS_WARNING("Failed to close file handle for current process!");
     }
 #else // XP_WIN
     HANDLE_EINTR(close(mHandle));
 #endif
     mHandle = INVALID_HANDLE;
   }
 }
 
@@ -90,23 +90,26 @@ FileDescriptor::ShareTo(const FileDescri
 {
   PlatformHandleType newHandle;
 #ifdef XP_WIN
   if (IsValid()) {
     if (DuplicateHandle(GetCurrentProcess(), mHandle, aOtherProcess,
                         &newHandle, 0, FALSE, DUPLICATE_SAME_ACCESS)) {
       return newHandle;
     }
-    NS_WARNING("Failed to duplicate file handle!");
+    NS_WARNING("Failed to duplicate file handle for other process!");
   }
   return INVALID_HANDLE;
 #else // XP_WIN
   if (IsValid()) {
     newHandle = dup(mHandle);
-    return base::FileDescriptor(newHandle, /* auto_close */ true);
+    if (IsValid(newHandle)) {
+      return base::FileDescriptor(newHandle, /* auto_close */ true);
+    }
+    NS_WARNING("Failed to duplicate file handle for other process!");
   }
   return base::FileDescriptor();
 #endif
 
   MOZ_NOT_REACHED("Must not get here!");
   return PickleType();
 }
 
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@@ -4321,17 +4321,20 @@ class _GenerateProtocolActorCode(ipdl.as
 
         read = MethodDefn(self.readMethodDecl(outtype, var))
         ifread = StmtIf(ExprNot(ExprCall(ExprVar('IPC::ReadParam'),
                                          args=[ msgvar, itervar,
                                                 ExprAddrOf(picklevar) ])))
         ifread.addifstmt(StmtReturn.FALSE)
 
         ifnvalid = StmtIf(ExprNot(ExprCall(ExprSelect(tmpvar, '.', 'IsValid'))))
-        ifnvalid.addifstmt(StmtReturn.FALSE)
+        ifnvalid.addifstmt(
+            _protocolErrorBreakpoint('[' +
+                                     _actorName(self.protocol.name, self.side) +
+                                     '] Received an invalid file descriptor!'))
 
         read.addstmts([
             StmtDecl(Decl(_fdPickleType(), picklevar.name)),
             ifread,
             Whitespace.NL,
             StmtDecl(Decl(_fdType(), tmpvar.name),
                      init=ExprCall(ExprVar('FileDescriptor'),
                                    args=[ _fdBackstagePass(), picklevar ])),
--- a/netwerk/ipc/RemoteOpenFileChild.cpp
+++ b/netwerk/ipc/RemoteOpenFileChild.cpp
@@ -138,16 +138,21 @@ RemoteOpenFileChild::AsyncRemoteFileOpen
 //-----------------------------------------------------------------------------
 
 bool
 RemoteOpenFileChild::RecvFileOpened(const FileDescriptor& aFD)
 {
 #if defined(XP_WIN) || defined(MOZ_WIDGET_COCOA)
   NS_NOTREACHED("osX and Windows shouldn't be doing IPDL here");
 #else
+  if (!aFD.IsValid()) {
+    return RecvFileDidNotOpen();
+  }
+
+  MOZ_ASSERT(!mNSPRFileDesc);
   mNSPRFileDesc = PR_AllocFileDesc(aFD.PlatformHandle(), PR_GetFileMethods());
 
   MOZ_ASSERT(mListener);
   mListener->OnRemoteFileOpenComplete(NS_OK);
   mListener = nullptr;     // release ref to listener
 
   // This calls NeckoChild::DeallocPRemoteOpenFile(), which deletes |this| if
   // IPDL holds the last reference.  Don't rely on |this| existing after here!
@@ -158,18 +163,20 @@ RemoteOpenFileChild::RecvFileOpened(cons
 }
 
 bool
 RemoteOpenFileChild::RecvFileDidNotOpen()
 {
 #if defined(XP_WIN) || defined(MOZ_WIDGET_COCOA)
   NS_NOTREACHED("osX and Windows shouldn't be doing IPDL here");
 #else
+  MOZ_ASSERT(!mNSPRFileDesc);
+  printf_stderr("RemoteOpenFileChild: file was not opened!\n");
+
   MOZ_ASSERT(mListener);
-  printf_stderr("RemoteOpenFileChild: file was not opened!\n");
   mListener->OnRemoteFileOpenComplete(NS_ERROR_FILE_NOT_FOUND);
   mListener = nullptr;     // release ref to listener
 
   // This calls NeckoChild::DeallocPRemoteOpenFile(), which deletes |this| if
   // IPDL holds the last reference.  Don't rely on |this| existing after here!
   Send__delete__(this);
 #endif