Bug 831307 - Allow invalid file handles to not crash child processes. r=cjones.
authorBen Turner <bent.mozilla@gmail.com>
Thu, 24 Jan 2013 17:10:39 -0800
changeset 130491 c8a4e004e3dccb04a5478e4120e5a2a5b223612f
parent 130490 cc0081ab6c4aa4a1551e0b1abf8f0cc1e44ee068
child 130492 9b73d9410c6f1909ff76ed74e73cc45212898909
push id317
push userbbajaj@mozilla.com
push dateTue, 07 May 2013 01:20:33 +0000
treeherdermozilla-release@159a10910249 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscjones
bugs831307
milestone21.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 831307 - Allow invalid file handles to not crash child processes. r=cjones.
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
@@ -965,17 +965,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