Skip to content

Commit c1e46e9

Browse files
izbyshevgpshead
authored andcommitted
bpo-32777: Fix _Py_set_inheritable async-safety in subprocess (GH-5560)
Fix a rare but potential pre-exec child process deadlock in subprocess on POSIX systems when marking file descriptors inheritable on exec in the child process. This bug appears to have been introduced in 3.4 with the inheritable file descriptors support. This also changes Python/fileutils.c `set_inheritable` to use the "slow" two `fcntl` syscall path instead of the "fast" single `ioctl` syscall path when asked to be async signal safe (by way of being asked not to raise exceptions). `ioctl` is not a POSIX async-signal-safe approved function. ref: http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
1 parent 22864bc commit c1e46e9

File tree

4 files changed

+26
-7
lines changed

4 files changed

+26
-7
lines changed

Include/fileutils.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,9 @@ PyAPI_FUNC(int) _Py_get_inheritable(int fd);
152152
PyAPI_FUNC(int) _Py_set_inheritable(int fd, int inheritable,
153153
int *atomic_flag_works);
154154

155+
PyAPI_FUNC(int) _Py_set_inheritable_async_safe(int fd, int inheritable,
156+
int *atomic_flag_works);
157+
155158
PyAPI_FUNC(int) _Py_dup(int fd);
156159

157160
#ifndef MS_WINDOWS
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a rare but potential pre-exec child process deadlock in subprocess on
2+
POSIX systems when marking file descriptors inheritable on exec in the child
3+
process. This bug appears to have been introduced in 3.4.

Modules/_posixsubprocess.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ make_inheritable(PyObject *py_fds_to_keep, int errpipe_write)
169169
called. */
170170
continue;
171171
}
172-
if (_Py_set_inheritable((int)fd, 1, NULL) < 0)
172+
if (_Py_set_inheritable_async_safe((int)fd, 1, NULL) < 0)
173173
return -1;
174174
}
175175
return 0;
@@ -431,21 +431,21 @@ child_exec(char *const exec_array[],
431431
dup2() removes the CLOEXEC flag but we must do it ourselves if dup2()
432432
would be a no-op (issue #10806). */
433433
if (p2cread == 0) {
434-
if (_Py_set_inheritable(p2cread, 1, NULL) < 0)
434+
if (_Py_set_inheritable_async_safe(p2cread, 1, NULL) < 0)
435435
goto error;
436436
}
437437
else if (p2cread != -1)
438438
POSIX_CALL(dup2(p2cread, 0)); /* stdin */
439439

440440
if (c2pwrite == 1) {
441-
if (_Py_set_inheritable(c2pwrite, 1, NULL) < 0)
441+
if (_Py_set_inheritable_async_safe(c2pwrite, 1, NULL) < 0)
442442
goto error;
443443
}
444444
else if (c2pwrite != -1)
445445
POSIX_CALL(dup2(c2pwrite, 1)); /* stdout */
446446

447447
if (errwrite == 2) {
448-
if (_Py_set_inheritable(errwrite, 1, NULL) < 0)
448+
if (_Py_set_inheritable_async_safe(errwrite, 1, NULL) < 0)
449449
goto error;
450450
}
451451
else if (errwrite != -1)

Python/fileutils.c

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -913,6 +913,7 @@ _Py_stat(PyObject *path, struct stat *statbuf)
913913
}
914914

915915

916+
/* This function MUST be kept async-signal-safe on POSIX when raise=0. */
916917
static int
917918
get_inheritable(int fd, int raise)
918919
{
@@ -958,6 +959,8 @@ _Py_get_inheritable(int fd)
958959
return get_inheritable(fd, 1);
959960
}
960961

962+
963+
/* This function MUST be kept async-signal-safe on POSIX when raise=0. */
961964
static int
962965
set_inheritable(int fd, int inheritable, int raise, int *atomic_flag_works)
963966
{
@@ -1014,8 +1017,10 @@ set_inheritable(int fd, int inheritable, int raise, int *atomic_flag_works)
10141017
#else
10151018

10161019
#if defined(HAVE_SYS_IOCTL_H) && defined(FIOCLEX) && defined(FIONCLEX)
1017-
if (ioctl_works != 0) {
1020+
if (ioctl_works != 0 && raise != 0) {
10181021
/* fast-path: ioctl() only requires one syscall */
1022+
/* caveat: raise=0 is an indicator that we must be async-signal-safe
1023+
* thus avoid using ioctl() so we skip the fast-path. */
10191024
if (inheritable)
10201025
request = FIONCLEX;
10211026
else
@@ -1086,8 +1091,7 @@ make_non_inheritable(int fd)
10861091
}
10871092

10881093
/* Set the inheritable flag of the specified file descriptor.
1089-
On success: return 0, on error: raise an exception if raise is nonzero
1090-
and return -1.
1094+
On success: return 0, on error: raise an exception and return -1.
10911095
10921096
If atomic_flag_works is not NULL:
10931097
@@ -1108,6 +1112,15 @@ _Py_set_inheritable(int fd, int inheritable, int *atomic_flag_works)
11081112
return set_inheritable(fd, inheritable, 1, atomic_flag_works);
11091113
}
11101114

1115+
/* Same as _Py_set_inheritable() but on error, set errno and
1116+
don't raise an exception.
1117+
This function is async-signal-safe. */
1118+
int
1119+
_Py_set_inheritable_async_safe(int fd, int inheritable, int *atomic_flag_works)
1120+
{
1121+
return set_inheritable(fd, inheritable, 0, atomic_flag_works);
1122+
}
1123+
11111124
static int
11121125
_Py_open_impl(const char *pathname, int flags, int gil_held)
11131126
{

0 commit comments

Comments
 (0)