8000 bpo-32844: Fix a subprocess misredirection of a low fd (GH5689) · python/cpython@57db13e · GitHub
[go: up one dir, main page]

Skip to content

Commit 57db13e

Browse files
bpo-32844: Fix a subprocess misredirection of a low fd (GH5689)
bpo-32844: subprocess: Fix a potential misredirection of a low fd to stderr. When redirecting, subprocess attempts to achieve the following state: each fd to be redirected to is less than or equal to the fd it is redirected from, which is necessary because redirection occurs in the ascending order of destination descriptors. It fails to do so in a couple of corner cases, for example, if 1 is redirected to 2 and 0 is closed in the parent. (cherry picked from commit 0e7144b) Co-authored-by: Alexey Izbyshev <izbyshev@users.noreply.github.com>
1 parent a413a91 commit 57db13e

File tree

3 files changed

+53
-1
lines changed

3 files changed

+53
-1
lines changed

Lib/test/test_subprocess.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import platform
77
import signal
88
import io
9+
import itertools
910
import os
1011
import errno
1112
import tempfile
@@ -2108,6 +2109,55 @@ def test_swap_fds(self):
21082109
self.check_swap_fds(2, 0, 1)
21092110
self.check_swap_fds(2, 1, 0)
21102111

2112+
def _check_swap_std_fds_with_one_closed(self, from_fds, to_fds):
2113+
saved_fds = self._save_fds(range(3))
2114+
try:
2115+
for from_fd in from_fds:
2116+
with tempfile.TemporaryFile() as f:
2117+
os.dup2(f.fileno(), from_fd)
2118+
2119+
fd_to_close = (set(range(3)) - set(from_fds)).pop()
2120+
os.close(fd_to_close)
2121+
2122+
arg_names = ['stdin', 'stdout', 'stderr']
2123+
kwargs = {}
2124+
for from_fd, to_fd in zip(from_fds, to_fds):
2125+
kwargs[arg_names[to_fd]] = from_fd
2126+
2127+
code = textwrap.dedent(r'''
2128+
import os, sys
2129+
skipped_fd = int(sys.argv[1])
2130+
for fd in range(3):
2131+
if fd != skipped_fd:
2132+
os.write(fd, str(fd).encode('ascii'))
2133+
''')
2134+
2135+
skipped_fd = (set(range(3)) - set(to_fds)).pop()
2136+
2137+
rc = subprocess.call([sys.executable, '-c', code, str(skipped_fd)],
2138+
**kwargs)
2139+
self.assertEqual(rc, 0)
2140+
2141+
for from_fd, to_fd in zip(from_fds, to_fds):
2142+
os.lseek(from_fd, 0, os.SEEK_SET)
2143+
read_bytes = os.read(from_fd, 1024)
2144+
read_fds = list(map(int, read_bytes.decode('ascii')))
2145+
msg = textwrap.dedent(f"""
2146+
When testing {from_fds} to {to_fds} redirection,
2147+
parent descriptor {from_fd} got redirected
2148+
to descriptor(s) {read_fds} instead of descriptor {to_fd}.
2149+
""")
2150+
self.assertEqual([to_fd], read_fds, msg)
2151+
finally:
2152+
self._restore_fds(saved_fds)
2153+
2154+
# Check that subprocess can remap std fds correctly even
2155+
# if one of them is closed (#32844).
2156+
def test_swap_std_fds_with_one_closed(self):
2157+
for from_fds in itertools.combinations(range(3), 2):
2158+
for to_fds in itertools.permutations(range(3), 2):
2159+
self._check_swap_std_fds_with_one_closed(from_fds, to_fds)
2160+
21112161
def test_surrogates_error_message(self):
21122162
def prepare():
21132163
raise ValueError("surrogate:\uDCff")
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix wrong redirection of a low descriptor (0 or 1) to stderr in subprocess
2+
if another low descriptor is closed.

Modules/_posixsubprocess.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ child_exec(char *const exec_array[],
424424
either 0, 1 or 2, it is possible that it is overwritten (#12607). */
425425
if (c2pwrite == 0)
426426
POSIX_CALL(c2pwrite = dup(c2pwrite));
427-
if (errwrite == 0 || errwrite == 1)
427+
while (errwrite == 0 || errwrite == 1)
428428
POSIX_CALL(errwrite = dup(errwrite));
429429

430430
/* Dup fds for child.

0 commit comments

Comments
 (0)
0