From b5461b9884a1a83ac5c1d03a0f3060b36b695a48 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 15 Jun 2013 18:04:26 -0700 Subject: Prevent a possible double close of parent pipe fds when the subprocess exec runs into an error. Prevent a regular multi-close of the /dev/null fd when any of stdin, stdout and stderr was set to DEVNULL. --- Lib/subprocess.py | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 689046e..a62d9a4 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -810,6 +810,7 @@ class Popen(object): if universal_newlines: self.stderr = io.TextIOWrapper(self.stderr) + self._closed_child_pipe_fds = False try: self._execute_child(args, executable, preexec_fn, close_fds, pass_fds, cwd, env, @@ -826,19 +827,21 @@ class Popen(object): except EnvironmentError: pass # Ignore EBADF or other errors. - # Make sure the child pipes are closed as well. - to_close = [] - if stdin == PIPE: - to_close.append(p2cread) - if stdout == PIPE: - to_close.append(c2pwrite) - if stderr == PIPE: - to_close.append(errwrite) - for fd in to_close: - try: - os.close(fd) - except EnvironmentError: - pass + if not self._closed_child_pipe_fds: + to_close = [] + if stdin == PIPE: + to_close.append(p2cread) + if stdout == PIPE: + to_close.append(c2pwrite) + if stderr == PIPE: + to_close.append(errwrite) + if hasattr(self, '_devnull'): + to_close.append(self._devnull) + for fd in to_close: + try: + os.close(fd) + except EnvironmentError: + pass raise @@ -1383,14 +1386,18 @@ class Popen(object): # be sure the FD is closed no matter what os.close(errpipe_write) - if p2cread != -1 and p2cwrite != -1: + # self._devnull is not always defined. + devnull_fd = getattr(self, '_devnull', None) + if p2cread != -1 and p2cwrite != -1 and p2cread != devnull_fd: os.close(p2cread) - if c2pwrite != -1 and c2pread != -1: + if c2pwrite != -1 and c2pread != -1 and c2pwrite != devnull_fd: os.close(c2pwrite) - if errwrite != -1 and errread != -1: + if errwrite != -1 and errread != -1 and errwrite != devnull_fd: os.close(errwrite) - if hasattr(self, '_devnull'): - os.close(self._devnull) + if devnull_fd is not None: + os.close(devnull_fd) + # Prevent a double close of these fds from __init__ on error. + self._closed_child_pipe_fds = True # Wait for exec to fail or succeed; possibly raising an # exception (limited in size) -- cgit v0.12 From 893f2ffc7c8a110f069bb05c66e60632cc49cbef Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 15 Jun 2013 18:05:17 -0700 Subject: news entry for subprocess double close fix. --- Misc/NEWS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Misc/NEWS b/Misc/NEWS index aacf0e6..67bc8dc 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -32,6 +32,10 @@ Core and Builtins Library ------- +- subprocess: Prevent a possible double close of parent pipe fds when the + subprocess exec runs into an error. Prevent a regular multi-close of the + /dev/null fd when any of stdin, stdout and stderr was set to DEVNULL. + - Issue #16102: Make uuid._netbios_getnode() work again on Python 3. - Issue #18109: os.uname() now decodes fields from the locale encoding, and -- cgit v0.12