Bug 1276386 - Prevent processes from inheriting extra file descriptors on Windows. r=mhowell
authorKris Maglione <maglione.k@gmail.com>
Sat, 28 May 2016 12:28:30 -0700
changeset 340682 03d6c612af0ddbf22f4fbe12ad034831be38fab7
parent 340681 a05fae3adc541c24e86692bd87d69c9acaf6aa6c
child 340683 f43900d9942fbcc1b818775f61f481b54a109120
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmhowell
bugs1276386
milestone49.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 1276386 - Prevent processes from inheriting extra file descriptors on Windows. r=mhowell MozReview-Commit-ID: IFi2Z7sqaxW
toolkit/modules/subprocess/subprocess_shared_win.js
toolkit/modules/subprocess/subprocess_worker_win.js
toolkit/modules/subprocess/test/xpcshell/test_subprocess.js
--- a/toolkit/modules/subprocess/subprocess_shared_win.js
+++ b/toolkit/modules/subprocess/subprocess_shared_win.js
@@ -8,20 +8,22 @@
 /* exported LIBC, Win, createPipe, libc */
 
 const LIBC = OS.Constants.libc;
 
 const Win = OS.Constants.Win;
 
 const LIBC_CHOICES = ["kernel32.dll"];
 
-const win32 = {
+var win32 = {
   // On Windows 64, winapi_abi is an alias for default_abi.
   WINAPI: ctypes.winapi_abi,
 
+  VOID: ctypes.void_t,
+
   BYTE: ctypes.uint8_t,
   WORD: ctypes.uint16_t,
   DWORD: ctypes.uint32_t,
 
   UINT: ctypes.unsigned_int,
   UCHAR: ctypes.unsigned_char,
 
   BOOL: ctypes.bool,
@@ -29,59 +31,72 @@ const win32 = {
   HANDLE: ctypes.voidptr_t,
   PVOID: ctypes.voidptr_t,
   LPVOID: ctypes.voidptr_t,
 
   CHAR: ctypes.char,
   WCHAR: ctypes.jschar,
 
   ULONG_PTR: ctypes.uintptr_t,
+
+  SIZE_T: ctypes.size_t,
+  PSIZE_T: ctypes.size_t.ptr,
 };
 
 Object.assign(win32, {
+  DWORD_PTR: win32.ULONG_PTR,
+
   LPSTR: win32.CHAR.ptr,
   LPWSTR: win32.WCHAR.ptr,
 
   LPBYTE: win32.BYTE.ptr,
   LPDWORD: win32.DWORD.ptr,
   LPHANDLE: win32.HANDLE.ptr,
+
+  // This is an opaque type.
+  PROC_THREAD_ATTRIBUTE_LIST: ctypes.char.array(),
+  LPPROC_THREAD_ATTRIBUTE_LIST: ctypes.char.ptr,
 });
 
 Object.assign(win32, {
   LPCSTR: win32.LPSTR,
   LPCWSTR: win32.LPWSTR,
   LPCVOID: win32.LPVOID,
 });
 
 Object.assign(win32, {
   CREATE_NEW_CONSOLE: 0x00000010,
   CREATE_UNICODE_ENVIRONMENT: 0x00000400,
+  EXTENDED_STARTUPINFO_PRESENT: 0x00080000,
   CREATE_NO_WINDOW: 0x08000000,
 
   STARTF_USESTDHANDLES: 0x0100,
 
   DUPLICATE_CLOSE_SOURCE: 0x01,
   DUPLICATE_SAME_ACCESS: 0x02,
 
   ERROR_HANDLE_EOF: 38,
   ERROR_BROKEN_PIPE: 109,
+  ERROR_INSUFFICIENT_BUFFER: 122,
 
   FILE_FLAG_OVERLAPPED: 0x40000000,
 
   PIPE_TYPE_BYTE: 0x00,
 
   PIPE_ACCESS_INBOUND: 0x01,
   PIPE_ACCESS_OUTBOUND: 0x02,
   PIPE_ACCESS_DUPLEX: 0x03,
 
   PIPE_WAIT: 0x00,
   PIPE_NOWAIT: 0x01,
 
   STILL_ACTIVE: 259,
 
+  PROC_THREAD_ATTRIBUTE_HANDLE_LIST: 0x00020002,
+
   // These constants are 32-bit unsigned integers, but Windows defines
   // them as negative integers cast to an unsigned type.
   STD_INPUT_HANDLE: -10 + 0x100000000,
   STD_OUTPUT_HANDLE: -11 + 0x100000000,
   STD_ERROR_HANDLE: -12 + 0x100000000,
 
   WAIT_TIMEOUT: 0x00000102,
   WAIT_FAILED: 0xffffffff,
@@ -126,16 +141,24 @@ Object.assign(win32, {
     {"cbReserved2": win32.WORD},
     {"lpReserved2": win32.LPBYTE},
     {"hStdInput": win32.HANDLE},
     {"hStdOutput": win32.HANDLE},
     {"hStdError": win32.HANDLE},
   ]),
 });
 
+Object.assign(win32, {
+  STARTUPINFOEXW: new ctypes.StructType("STARTUPINFOEXW", [
+    {"StartupInfo": win32.STARTUPINFOW},
+    {"lpAttributeList": win32.LPPROC_THREAD_ATTRIBUTE_LIST},
+  ]),
+});
+
+
 var libc = new Library("libc", LIBC_CHOICES, {
   CloseHandle: [
     win32.WINAPI,
     win32.BOOL,
     win32.HANDLE, /* hObject */
   ],
 
   CreateEventW: [
@@ -191,16 +214,22 @@ var libc = new Library("libc", LIBC_CHOI
     win32.BOOL, /* bInheritHandle */
     win32.DWORD, /* dwCreationFlags */
     win32.LPVOID, /* opt lpEnvironment */
     win32.LPCWSTR, /* opt lpCurrentDirectory */
     win32.STARTUPINFOW.ptr, /* lpStartupInfo */
     win32.PROCESS_INFORMATION.ptr, /* out lpProcessInformation */
   ],
 
+  DeleteProcThreadAttributeList: [
+    win32.WINAPI,
+    win32.VOID,
+    win32.LPPROC_THREAD_ATTRIBUTE_LIST, /* in/out lpAttributeList */
+  ],
+
   DuplicateHandle: [
     win32.WINAPI,
     win32.BOOL,
     win32.HANDLE, /* hSourceProcessHandle */
     win32.HANDLE, /* hSourceHandle */
     win32.HANDLE, /* hTargetProcessHandle */
     win32.LPHANDLE, /* out lpTargetHandle */
     win32.DWORD, /* dwDesiredAccess */
@@ -246,16 +275,25 @@ var libc = new Library("libc", LIBC_CHOI
   ],
 
   GetStdHandle: [
     win32.WINAPI,
     win32.HANDLE,
     win32.DWORD, /* nStdHandle */
   ],
 
+  InitializeProcThreadAttributeList: [
+    win32.WINAPI,
+    win32.BOOL,
+    win32.LPPROC_THREAD_ATTRIBUTE_LIST, /* out opt lpAttributeList */
+    win32.DWORD, /* dwAttributeCount */
+    win32.DWORD, /* dwFlags */
+    win32.PSIZE_T, /* in/out lpSize */
+  ],
+
   ReadFile: [
     win32.WINAPI,
     win32.BOOL,
     win32.HANDLE, /* hFile */
     win32.LPVOID, /* out lpBuffer */
     win32.DWORD, /* nNumberOfBytesToRead */
     win32.LPDWORD, /* opt out lpNumberOfBytesRead */
     win32.OVERLAPPED.ptr, /* opt in/out lpOverlapped */
@@ -263,16 +301,28 @@ var libc = new Library("libc", LIBC_CHOI
 
   TerminateProcess: [
     win32.WINAPI,
     win32.BOOL,
     win32.HANDLE, /* hProcess */
     win32.UINT, /* uExitCode */
   ],
 
+  UpdateProcThreadAttribute: [
+    win32.WINAPI,
+    win32.BOOL,
+    win32.LPPROC_THREAD_ATTRIBUTE_LIST, /* in/out lpAttributeList */
+    win32.DWORD, /* dwFlags */
+    win32.DWORD_PTR, /* Attribute */
+    win32.PVOID, /* lpValue */
+    win32.SIZE_T, /* cbSize */
+    win32.PVOID, /* out opt lpPreviousValue */
+    win32.PSIZE_T, /* opt lpReturnSize */
+  ],
+
   WaitForMultipleObjects: [
     win32.WINAPI,
     win32.DWORD,
     win32.DWORD, /* nCount */
     win32.HANDLE.ptr, /* hHandles */
     win32.BOOL, /* bWaitAll */
     win32.DWORD, /* dwMilliseconds */
   ],
@@ -336,8 +386,42 @@ win32.createPipe = function(secAttr, rea
   if (isInvalid(writeHandle)) {
     libc.CloseHandle(readHandle);
     return [];
   }
 
   return [win32.Handle(readHandle),
           win32.Handle(writeHandle)];
 };
+
+win32.createThreadAttributeList = function(handles) {
+  try {
+    void libc.InitializeProcThreadAttributeList;
+    void libc.DeleteProcThreadAttributeList;
+    void libc.UpdateProcThreadAttribute;
+  } catch (e) {
+    // This is only supported in Windows Vista and later.
+    return null;
+  }
+
+  let size = win32.SIZE_T();
+  if (!libc.InitializeProcThreadAttributeList(null, 1, 0, size.address()) &&
+      ctypes.winLastError != win32.ERROR_INSUFFICIENT_BUFFER) {
+    return null;
+  }
+
+  let attrList = win32.PROC_THREAD_ATTRIBUTE_LIST(size.value);
+
+  if (!libc.InitializeProcThreadAttributeList(attrList, 1, 0, size.address())) {
+    return null;
+  }
+
+  let ok = libc.UpdateProcThreadAttribute(
+    attrList, 0, win32.PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
+    handles, handles.constructor.size, null, null);
+
+  if (!ok) {
+    libc.DeleteProcThreadAttributeList(attrList);
+    return null;
+  }
+
+  return attrList;
+};
--- a/toolkit/modules/subprocess/subprocess_worker_win.js
+++ b/toolkit/modules/subprocess/subprocess_worker_win.js
@@ -420,39 +420,58 @@ class Process extends BaseProcess {
 
     let envp = this.stringList(options.environment);
 
     let handles = this.initPipes(options);
 
     let processFlags = win32.CREATE_NO_WINDOW
                      | win32.CREATE_UNICODE_ENVIRONMENT;
 
-    let startupInfo = new win32.STARTUPINFOW();
+    let startupInfoEx = new win32.STARTUPINFOEXW();
+    let startupInfo = startupInfoEx.StartupInfo;
+
     startupInfo.cb = win32.STARTUPINFOW.size;
     startupInfo.dwFlags = win32.STARTF_USESTDHANDLES;
 
     startupInfo.hStdInput = handles[0];
     startupInfo.hStdOutput = handles[1];
     startupInfo.hStdError = handles[2];
 
+    // Note: This needs to be kept alive until we destroy the attribute list.
+    let handleArray = win32.HANDLE.array()(handles);
+
+    let threadAttrs = win32.createThreadAttributeList(handleArray);
+    if (threadAttrs) {
+      // If have thread attributes to pass, pass the size of the full extended
+      // startup info struct.
+      processFlags |= win32.EXTENDED_STARTUPINFO_PRESENT;
+      startupInfo.cb = win32.STARTUPINFOEXW.size;
+
+      startupInfoEx.lpAttributeList = threadAttrs;
+    }
+
     let procInfo = new win32.PROCESS_INFORMATION();
 
     let ok = libc.CreateProcessW(
       command, args.join(" "),
       null, /* Security attributes */
       null, /* Thread security attributes */
       true, /* Inherits handles */
       processFlags, envp, options.workdir,
       startupInfo.address(),
       procInfo.address());
 
     for (let handle of new Set(handles)) {
       handle.dispose();
     }
 
+    if (threadAttrs) {
+      libc.DeleteProcThreadAttributeList(threadAttrs);
+    }
+
     if (!ok) {
       for (let pipe of this.pipes) {
         pipe.close();
       }
       throw new Error("Failed to create process");
     }
 
     libc.CloseHandle(procInfo.hThread);
--- a/toolkit/modules/subprocess/test/xpcshell/test_subprocess.js
+++ b/toolkit/modules/subprocess/test/xpcshell/test_subprocess.js
@@ -408,16 +408,54 @@ add_task(function* test_subprocess_inval
     "Promise should be rejected on EOF");
 
   let {exitCode} = yield proc.wait();
 
   equal(exitCode, 0, "Got expected exit code");
 });
 
 
+if (AppConstants.isPlatformAndVersionAtLeast("win", "6")) {
+  add_task(function* test_subprocess_inherited_descriptors() {
+    let {ctypes, libc, win32} = Cu.import("resource://gre/modules/subprocess/subprocess_win.jsm");
+
+    let secAttr = new win32.SECURITY_ATTRIBUTES();
+    secAttr.nLength = win32.SECURITY_ATTRIBUTES.size;
+    secAttr.bInheritHandle = true;
+
+    let handles = win32.createPipe(secAttr, 0);
+
+
+    let proc = yield Subprocess.call({
+      command: PYTHON,
+      arguments: ["-u", TEST_SCRIPT, "echo"],
+    });
+
+
+    // Close the output end of the pipe.
+    // Ours should be the only copy, so reads should fail after this.
+    handles[1].dispose();
+
+    let buffer = new ArrayBuffer(1);
+    let succeeded = libc.ReadFile(handles[0], buffer, buffer.byteLength,
+                                  null, null);
+
+    ok(!succeeded, "ReadFile should fail on broken pipe");
+    equal(ctypes.winLastError, win32.ERROR_BROKEN_PIPE, "Read should fail with ERROR_BROKEN_PIPE");
+
+
+    proc.stdin.close();
+
+    let {exitCode} = yield proc.wait();
+
+    equal(exitCode, 0, "Got expected exit code");
+  });
+}
+
+
 add_task(function* test_subprocess_wait() {
   let proc = yield Subprocess.call({
     command: PYTHON,
     arguments: ["-u", TEST_SCRIPT, "exit", "42"],
   });
 
   let {exitCode} = yield proc.wait();