diff --git a/backport-bpo-35823-Allow-setsid-after-vfork-on-Linux.-GH-2294.patch b/backport-bpo-35823-Allow-setsid-after-vfork-on-Linux.-GH-2294.patch new file mode 100644 index 0000000000000000000000000000000000000000..5f0731d44941902d9f1769357b2931de8869f0db --- /dev/null +++ b/backport-bpo-35823-Allow-setsid-after-vfork-on-Linux.-GH-2294.patch @@ -0,0 +1,51 @@ +From be3c3a0e468237430ad7d19a33c60d306199a7f2 Mon Sep 17 00:00:00 2001 +From: "Gregory P. Smith" +Date: Sat, 24 Oct 2020 12:07:35 -0700 +Subject: [PATCH] bpo-35823: Allow setsid() after vfork() on Linux. (GH-22945) + +It should just be a syscall updating a couple of fields in the kernel side +process info. Confirming, in glibc is appears to be a shim for the setsid +syscall (based on not finding any code implementing anything special for it) +and in uclibc (*much* easier to read) it is clearly just a setsid syscall shim. + +A breadcrumb _suggesting_ that it is not allowed on Darwin/macOS comes from +a commit in emacs: https://lists.gnu.org/archive/html/bug-gnu-emacs/2017-04/msg00297.html +but I don't have a way to verify if that is true or not. +As we are not supporting vfork on macOS today I just left a note in a comment. +--- + Modules/_posixsubprocess.c | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c +index b7cba30..8baea31 100644 +--- a/Modules/_posixsubprocess.c ++++ b/Modules/_posixsubprocess.c +@@ -38,6 +38,8 @@ + + #if defined(__linux__) && defined(HAVE_VFORK) && defined(HAVE_SIGNAL_H) && \ + defined(HAVE_PTHREAD_SIGMASK) && !defined(HAVE_BROKEN_PTHREAD_SIGMASK) ++/* If this is ever expanded to non-Linux platforms, verify what calls are ++ * allowed after vfork(). Ex: setsid() may be disallowed on macOS? */ + # include + # define VFORK_USABLE 1 + #endif +@@ -712,7 +714,6 @@ struct linux_dirent64 { + #ifdef VFORK_USABLE + if (child_sigmask) { + /* These are checked by our caller; verify them in debug builds. */ +- assert(!call_setsid); + assert(!call_setuid); + assert(!call_setgid); + assert(!call_setgroups); +@@ -997,7 +998,7 @@ struct linux_dirent64 { + /* Use vfork() only if it's safe. See the comment above child_exec(). */ + sigset_t old_sigs; + if (preexec_fn == Py_None && +- !call_setuid && !call_setgid && !call_setgroups && !call_setsid) { ++ !call_setuid && !call_setgid && !call_setgroups) { + /* Block all signals to ensure that no signal handlers are run in the + * child process while it shares memory with us. Note that signals + * used internally by C libraries won't be blocked by +-- +1.8.3.1 + diff --git a/backport-bpo-35823-subprocess-Fix-handling-of-pthread_sigmask.patch b/backport-bpo-35823-subprocess-Fix-handling-of-pthread_sigmask.patch new file mode 100644 index 0000000000000000000000000000000000000000..c8f743f59b821f7711ccba25f9d8e20db1e9d37f --- /dev/null +++ b/backport-bpo-35823-subprocess-Fix-handling-of-pthread_sigmask.patch @@ -0,0 +1,63 @@ +From 473db47747bb8bc986d88ad81799bcbd88153ac5 Mon Sep 17 00:00:00 2001 +From: Alexey Izbyshev +Date: Sat, 24 Oct 2020 20:47:38 +0300 +Subject: [PATCH] bpo-35823: subprocess: Fix handling of pthread_sigmask() + errors (GH-22944) + +Using POSIX_CALL() is incorrect since pthread_sigmask() returns +the error number instead of setting errno. + +Also handle failure of the first call to pthread_sigmask() +in the parent process, and explain why we don't handle failure +of the second call in a comment. +--- + Modules/_posixsubprocess.c | 19 +++++++++++++++---- + 1 file changed, 15 insertions(+), 4 deletions(-) + +diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c +index ed49857..b7cba30 100644 +--- a/Modules/_posixsubprocess.c ++++ b/Modules/_posixsubprocess.c +@@ -581,7 +581,9 @@ struct linux_dirent64 { + #ifdef VFORK_USABLE + if (child_sigmask) { + reset_signal_handlers(child_sigmask); +- POSIX_CALL(pthread_sigmask(SIG_SETMASK, child_sigmask, NULL)); ++ if ((errno = pthread_sigmask(SIG_SETMASK, child_sigmask, NULL))) { ++ goto error; ++ } + } + #endif + +@@ -1007,7 +1009,11 @@ struct linux_dirent64 { + */ + sigset_t all_sigs; + sigfillset(&all_sigs); +- pthread_sigmask(SIG_BLOCK, &all_sigs, &old_sigs); ++ if ((saved_errno = pthread_sigmask(SIG_BLOCK, &all_sigs, &old_sigs))) { ++ errno = saved_errno; ++ PyErr_SetFromErrno(PyExc_OSError); ++ goto cleanup; ++ } + old_sigmask = &old_sigs; + } + #endif +@@ -1034,8 +1040,13 @@ struct linux_dirent64 { + * Note that in environments where vfork() is implemented as fork(), + * such as QEMU user-mode emulation, the parent won't be blocked, + * but it won't share the address space with the child, +- * so it's still safe to unblock the signals. */ +- pthread_sigmask(SIG_SETMASK, old_sigmask, NULL); ++ * so it's still safe to unblock the signals. ++ * ++ * We don't handle errors here because this call can't fail ++ * if valid arguments are given, and because there is no good ++ * way for the caller to deal with a failure to restore ++ * the thread signal mask. */ ++ (void) pthread_sigmask(SIG_SETMASK, old_sigmask, NULL); + } + #endif + +-- +1.8.3.1 + diff --git a/backport-bpo-35823-subprocess-Use-vfork-instead-of-fork-on-Li.patch b/backport-bpo-35823-subprocess-Use-vfork-instead-of-fork-on-Li.patch new file mode 100644 index 0000000000000000000000000000000000000000..efdfd59ac6abe63dba3eeb3dae19860c23cefefc --- /dev/null +++ b/backport-bpo-35823-subprocess-Use-vfork-instead-of-fork-on-Li.patch @@ -0,0 +1,417 @@ +From 976da903a746a5455998e9ca45fbc4d3ad3479d8 Mon Sep 17 00:00:00 2001 +From: Alexey Izbyshev +Date: Sat, 24 Oct 2020 03:47:01 +0300 +Subject: [PATCH] bpo-35823: subprocess: Use vfork() instead of fork() on Linux + when safe (GH-11671) + +* bpo-35823: subprocess: Use vfork() instead of fork() on Linux when safe + +When used to run a new executable image, fork() is not a good choice +for process creation, especially if the parent has a large working set: +fork() needs to copy page tables, which is slow, and may fail on systems +where overcommit is disabled, despite that the child is not going to +touch most of its address space. + +Currently, subprocess is capable of using posix_spawn() instead, which +normally provides much better performance. However, posix_spawn() does not +support many of child setup operations exposed by subprocess.Popen(). +Most notably, it's not possible to express `close_fds=True`, which +happens to be the default, via posix_spawn(). As a result, most users +can't benefit from faster process creation, at least not without +changing their code. + +However, Linux provides vfork() system call, which creates a new process +without copying the address space of the parent, and which is actually +used by C libraries to efficiently implement posix_spawn(). Due to sharing +of the address space and even the stack with the parent, extreme care +is required to use vfork(). At least the following restrictions must hold: + +* No signal handlers must execute in the child process. Otherwise, they + might clobber memory shared with the parent, potentially confusing it. + +* Any library function called after vfork() in the child must be + async-signal-safe (as for fork()), but it must also not interact with any + library state in a way that might break due to address space sharing + and/or lack of any preparations performed by libraries on normal fork(). + POSIX.1 permits to call only execve() and _exit(), and later revisions + remove vfork() specification entirely. In practice, however, almost all + operations needed by subprocess.Popen() can be safely implemented on + Linux. + +* Due to sharing of the stack with the parent, the child must be careful + not to clobber local variables that are alive across vfork() call. + Compilers are normally aware of this and take extra care with vfork() + (and setjmp(), which has a similar problem). + +* In case the parent is privileged, special attention must be paid to vfork() + use, because sharing an address space across different privilege domains + is insecure[1]. + +This patch adds support for using vfork() instead of fork() on Linux +when it's possible to do safely given the above. In particular: + +* vfork() is not used if credential switch is requested. The reverse case + (simple subprocess.Popen() but another application thread switches + credentials concurrently) is not possible for pure-Python apps because + subprocess.Popen() and functions like os.setuid() are mutually excluded + via GIL. We might also consider to add a way to opt-out of vfork() (and + posix_spawn() on platforms where it might be implemented via vfork()) in + a future PR. + +* vfork() is not used if `preexec_fn != None`. + +With this change, subprocess will still use posix_spawn() if possible, but +will fallback to vfork() on Linux in most cases, and, failing that, +to fork(). + +[1] https://ewontfix.com/7 + +Co-authored-by: Gregory P. Smith [Google LLC] +--- + .../2020-10-16-07-45-35.bpo-35823.SNQo56.rst | 2 + + Modules/_posixsubprocess.c | 224 ++++++++++++++++++--- + configure | 2 +- + configure.ac | 2 +- + pyconfig.h.in | 3 + + 5 files changed, 204 insertions(+), 29 deletions(-) + create mode 100644 Misc/NEWS.d/next/Library/2020-10-16-07-45-35.bpo-35823.SNQo56.rst + +diff --git a/Misc/NEWS.d/next/Library/2020-10-16-07-45-35.bpo-35823.SNQo56.rst b/Misc/NEWS.d/next/Library/2020-10-16-07-45-35.bpo-35823.SNQo56.rst +new file mode 100644 +index 0000000..cd428d3 +--- /dev/null ++++ b/Misc/NEWS.d/next/Library/2020-10-16-07-45-35.bpo-35823.SNQo56.rst +@@ -0,0 +1,2 @@ ++Use ``vfork()`` instead of ``fork()`` for :func:`subprocess.Popen` on Linux ++to improve performance in cases where it is deemed safe. +diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c +index d08c479..ed49857 100644 +--- a/Modules/_posixsubprocess.c ++++ b/Modules/_posixsubprocess.c +@@ -36,6 +36,12 @@ + # define SYS_getdents64 __NR_getdents64 + #endif + ++#if defined(__linux__) && defined(HAVE_VFORK) && defined(HAVE_SIGNAL_H) && \ ++ defined(HAVE_PTHREAD_SIGMASK) && !defined(HAVE_BROKEN_PTHREAD_SIGMASK) ++# include ++# define VFORK_USABLE 1 ++#endif ++ + #if defined(__sun) && defined(__SVR4) + /* readdir64 is used to work around Solaris 9 bug 6395699. */ + # define readdir readdir64 +@@ -407,9 +413,53 @@ struct linux_dirent64 { + #endif /* else NOT (defined(__linux__) && defined(HAVE_SYS_SYSCALL_H)) */ + + ++#ifdef VFORK_USABLE ++/* Reset dispositions for all signals to SIG_DFL except for ignored ++ * signals. This way we ensure that no signal handlers can run ++ * after we unblock signals in a child created by vfork(). ++ */ ++static void ++reset_signal_handlers(const sigset_t *child_sigmask) ++{ ++ struct sigaction sa_dfl = {.sa_handler = SIG_DFL}; ++ for (int sig = 1; sig < _NSIG; sig++) { ++ /* Dispositions for SIGKILL and SIGSTOP can't be changed. */ ++ if (sig == SIGKILL || sig == SIGSTOP) { ++ continue; ++ } ++ ++ /* There is no need to reset the disposition of signals that will ++ * remain blocked across execve() since the kernel will do it. */ ++ if (sigismember(child_sigmask, sig) == 1) { ++ continue; ++ } ++ ++ struct sigaction sa; ++ /* C libraries usually return EINVAL for signals used ++ * internally (e.g. for thread cancellation), so simply ++ * skip errors here. */ ++ if (sigaction(sig, NULL, &sa) == -1) { ++ continue; ++ } ++ ++ /* void *h works as these fields are both pointer types already. */ ++ void *h = (sa.sa_flags & SA_SIGINFO ? (void *)sa.sa_sigaction : ++ (void *)sa.sa_handler); ++ if (h == SIG_IGN || h == SIG_DFL) { ++ continue; ++ } ++ ++ /* This call can't reasonably fail, but if it does, terminating ++ * the child seems to be too harsh, so ignore errors. */ ++ (void) sigaction(sig, &sa_dfl, NULL); ++ } ++} ++#endif /* VFORK_USABLE */ ++ ++ + /* +- * This function is code executed in the child process immediately after fork +- * to set things up and call exec(). ++ * This function is code executed in the child process immediately after ++ * (v)fork to set things up and call exec(). + * + * All of the code in this function must only use async-signal-safe functions, + * listed at `man 7 signal` or +@@ -417,8 +467,28 @@ struct linux_dirent64 { + * + * This restriction is documented at + * http://www.opengroup.org/onlinepubs/009695399/functions/fork.html. ++ * ++ * If this function is called after vfork(), even more care must be taken. ++ * The lack of preparations that C libraries normally take on fork(), ++ * as well as sharing the address space with the parent, might make even ++ * async-signal-safe functions vfork-unsafe. In particular, on Linux, ++ * set*id() and setgroups() library functions must not be called, since ++ * they have to interact with the library-level thread list and send ++ * library-internal signals to implement per-process credentials semantics ++ * required by POSIX but not supported natively on Linux. Another reason to ++ * avoid this family of functions is that sharing an address space between ++ * processes running with different privileges is inherently insecure. ++ * See bpo-35823 for further discussion and references. ++ * ++ * In some C libraries, setrlimit() has the same thread list/signalling ++ * behavior since resource limits were per-thread attributes before ++ * Linux 2.6.10. Musl, as of 1.2.1, is known to have this issue ++ * (https://www.openwall.com/lists/musl/2020/10/15/6). ++ * ++ * If vfork-unsafe functionality is desired after vfork(), consider using ++ * syscall() to obtain it. + */ +-static void ++_Py_NO_INLINE static void + child_exec(char *const exec_array[], + char *const argv[], + char *const envp[], +@@ -432,6 +502,7 @@ struct linux_dirent64 { + int call_setgid, gid_t gid, + int call_setgroups, size_t groups_size, const gid_t *groups, + int call_setuid, uid_t uid, int child_umask, ++ const void *child_sigmask, + PyObject *py_fds_to_keep, + PyObject *preexec_fn, + PyObject *preexec_fn_args_tuple) +@@ -507,6 +578,13 @@ struct linux_dirent64 { + if (restore_signals) + _Py_RestoreSignals(); + ++#ifdef VFORK_USABLE ++ if (child_sigmask) { ++ reset_signal_handlers(child_sigmask); ++ POSIX_CALL(pthread_sigmask(SIG_SETMASK, child_sigmask, NULL)); ++ } ++#endif ++ + #ifdef HAVE_SETSID + if (call_setsid) + POSIX_CALL(setsid()); +@@ -599,6 +677,81 @@ struct linux_dirent64 { + } + + ++/* The main purpose of this wrapper function is to isolate vfork() from both ++ * subprocess_fork_exec() and child_exec(). A child process created via ++ * vfork() executes on the same stack as the parent process while the latter is ++ * suspended, so this function should not be inlined to avoid compiler bugs ++ * that might clobber data needed by the parent later. Additionally, ++ * child_exec() should not be inlined to avoid spurious -Wclobber warnings from ++ * GCC (see bpo-35823). ++ */ ++_Py_NO_INLINE static pid_t ++do_fork_exec(char *const exec_array[], ++ char *const argv[], ++ char *const envp[], ++ const char *cwd, ++ int p2cread, int p2cwrite, ++ int c2pread, int c2pwrite, ++ int errread, int errwrite, ++ int errpipe_read, int errpipe_write, ++ int close_fds, int restore_signals, ++ int call_setsid, ++ int call_setgid, gid_t gid, ++ int call_setgroups, size_t groups_size, const gid_t *groups, ++ int call_setuid, uid_t uid, int child_umask, ++ const void *child_sigmask, ++ PyObject *py_fds_to_keep, ++ PyObject *preexec_fn, ++ PyObject *preexec_fn_args_tuple) ++{ ++ ++ pid_t pid; ++ ++#ifdef VFORK_USABLE ++ if (child_sigmask) { ++ /* These are checked by our caller; verify them in debug builds. */ ++ assert(!call_setsid); ++ assert(!call_setuid); ++ assert(!call_setgid); ++ assert(!call_setgroups); ++ assert(preexec_fn == Py_None); ++ ++ pid = vfork(); ++ } else ++#endif ++ { ++ pid = fork(); ++ } ++ ++ if (pid != 0) { ++ return pid; ++ } ++ ++ /* Child process. ++ * See the comment above child_exec() for restrictions imposed on ++ * the code below. ++ */ ++ ++ if (preexec_fn != Py_None) { ++ /* We'll be calling back into Python later so we need to do this. ++ * This call may not be async-signal-safe but neither is calling ++ * back into Python. The user asked us to use hope as a strategy ++ * to avoid deadlock... */ ++ PyOS_AfterFork_Child(); ++ } ++ ++ child_exec(exec_array, argv, envp, cwd, ++ p2cread, p2cwrite, c2pread, c2pwrite, ++ errread, errwrite, errpipe_read, errpipe_write, ++ close_fds, restore_signals, call_setsid, ++ call_setgid, gid, call_setgroups, groups_size, groups, ++ call_setuid, uid, child_umask, child_sigmask, ++ py_fds_to_keep, preexec_fn, preexec_fn_args_tuple); ++ _exit(255); ++ return 0; /* Dead code to avoid a potential compiler warning. */ ++} ++ ++ + static PyObject * + subprocess_fork_exec(PyObject* self, PyObject *args) + { +@@ -836,39 +989,56 @@ struct linux_dirent64 { + need_after_fork = 1; + } + +- pid = fork(); +- if (pid == 0) { +- /* Child process */ +- /* +- * Code from here to _exit() must only use async-signal-safe functions, +- * listed at `man 7 signal` or +- * http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html. ++ /* NOTE: When old_sigmask is non-NULL, do_fork_exec() may use vfork(). */ ++ const void *old_sigmask = NULL; ++#ifdef VFORK_USABLE ++ /* Use vfork() only if it's safe. See the comment above child_exec(). */ ++ sigset_t old_sigs; ++ if (preexec_fn == Py_None && ++ !call_setuid && !call_setgid && !call_setgroups && !call_setsid) { ++ /* Block all signals to ensure that no signal handlers are run in the ++ * child process while it shares memory with us. Note that signals ++ * used internally by C libraries won't be blocked by ++ * pthread_sigmask(), but signal handlers installed by C libraries ++ * normally service only signals originating from *within the process*, ++ * so it should be sufficient to consider any library function that ++ * might send such a signal to be vfork-unsafe and do not call it in ++ * the child. + */ ++ sigset_t all_sigs; ++ sigfillset(&all_sigs); ++ pthread_sigmask(SIG_BLOCK, &all_sigs, &old_sigs); ++ old_sigmask = &old_sigs; ++ } ++#endif + +- if (preexec_fn != Py_None) { +- /* We'll be calling back into Python later so we need to do this. +- * This call may not be async-signal-safe but neither is calling +- * back into Python. The user asked us to use hope as a strategy +- * to avoid deadlock... */ +- PyOS_AfterFork_Child(); +- } ++ pid = do_fork_exec(exec_array, argv, envp, cwd, ++ p2cread, p2cwrite, c2pread, c2pwrite, ++ errread, errwrite, errpipe_read, errpipe_write, ++ close_fds, restore_signals, call_setsid, ++ call_setgid, gid, call_setgroups, num_groups, groups, ++ call_setuid, uid, child_umask, old_sigmask, ++ py_fds_to_keep, preexec_fn, preexec_fn_args_tuple); + +- child_exec(exec_array, argv, envp, cwd, +- p2cread, p2cwrite, c2pread, c2pwrite, +- errread, errwrite, errpipe_read, errpipe_write, +- close_fds, restore_signals, call_setsid, +- call_setgid, gid, call_setgroups, num_groups, groups, +- call_setuid, uid, child_umask, +- py_fds_to_keep, preexec_fn, preexec_fn_args_tuple); +- _exit(255); +- return NULL; /* Dead code to avoid a potential compiler warning. */ +- } + /* Parent (original) process */ + if (pid == -1) { + /* Capture errno for the exception. */ + saved_errno = errno; + } + ++#ifdef VFORK_USABLE ++ if (old_sigmask) { ++ /* vfork() semantics guarantees that the parent is blocked ++ * until the child performs _exit() or execve(), so it is safe ++ * to unblock signals once we're here. ++ * Note that in environments where vfork() is implemented as fork(), ++ * such as QEMU user-mode emulation, the parent won't be blocked, ++ * but it won't share the address space with the child, ++ * so it's still safe to unblock the signals. */ ++ pthread_sigmask(SIG_SETMASK, old_sigmask, NULL); ++ } ++#endif ++ + Py_XDECREF(cwd_obj2); + + if (need_after_fork) +diff --git a/configure b/configure +index 29f33b5..bc87485 100755 +--- a/configure ++++ b/configure +@@ -11732,7 +11732,7 @@ for ac_func in alarm accept4 setitimer getitimer bind_textdomain_codeset chown \ + sigaction sigaltstack sigfillset siginterrupt sigpending sigrelse \ + sigtimedwait sigwait sigwaitinfo snprintf strftime strlcpy strsignal symlinkat sync \ + sysconf tcgetpgrp tcsetpgrp tempnam timegm times tmpfile tmpnam tmpnam_r \ +- truncate uname unlinkat utimensat utimes waitid waitpid wait3 wait4 \ ++ truncate uname unlinkat utimensat utimes vfork waitid waitpid wait3 wait4 \ + wcscoll wcsftime wcsxfrm wmemcmp writev _getpty rtpSpawn + do : + as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` +diff --git a/configure.ac b/configure.ac +index 9698c3c..49ed09a 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -3690,7 +3690,7 @@ AC_CHECK_FUNCS(alarm accept4 setitimer getitimer bind_textdomain_codeset chown \ + sigaction sigaltstack sigfillset siginterrupt sigpending sigrelse \ + sigtimedwait sigwait sigwaitinfo snprintf strftime strlcpy strsignal symlinkat sync \ + sysconf tcgetpgrp tcsetpgrp tempnam timegm times tmpfile tmpnam tmpnam_r \ +- truncate uname unlinkat utimensat utimes waitid waitpid wait3 wait4 \ ++ truncate uname unlinkat utimensat utimes vfork waitid waitpid wait3 wait4 \ + wcscoll wcsftime wcsxfrm wmemcmp writev _getpty rtpSpawn) + + # Force lchmod off for Linux. Linux disallows changing the mode of symbolic +diff --git a/pyconfig.h.in b/pyconfig.h.in +index 298cb4f..af8a3d6 100644 +--- a/pyconfig.h.in ++++ b/pyconfig.h.in +@@ -1301,6 +1301,9 @@ + /* Define to 1 if you have the header file. */ + #undef HAVE_UUID_UUID_H + ++/* Define to 1 if you have the `vfork' function. */ ++#undef HAVE_VFORK ++ + /* Define to 1 if you have the `wait3' function. */ + #undef HAVE_WAIT3 + +-- +1.8.3.1 + diff --git a/backport-bpo-42146-Unify-cleanup-in-subprocess_fork_exec-GH-2.patch b/backport-bpo-42146-Unify-cleanup-in-subprocess_fork_exec-GH-2.patch new file mode 100644 index 0000000000000000000000000000000000000000..8dadd6fbec7626681bea2fab4af395d5d9787769 --- /dev/null +++ b/backport-bpo-42146-Unify-cleanup-in-subprocess_fork_exec-GH-2.patch @@ -0,0 +1,136 @@ +From d3b4e068077dd26927ae7485bd0303e09d962c02 Mon Sep 17 00:00:00 2001 +From: Alexey Izbyshev +Date: Sun, 1 Nov 2020 08:33:08 +0300 +Subject: [PATCH] bpo-42146: Unify cleanup in subprocess_fork_exec() (GH-22970) + +* bpo-42146: Unify cleanup in subprocess_fork_exec() + +Also ignore errors from _enable_gc(): +* They are always suppressed by the current code due to a bug. +* _enable_gc() is only used if `preexec_fn != None`, which is unsafe. +* We don't have a good way to handle errors in case we successfully + created a child process. + +Co-authored-by: Gregory P. Smith +--- + Modules/_posixsubprocess.c | 53 ++++++++++++++++------------------------------ + 1 file changed, 18 insertions(+), 35 deletions(-) + +diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c +index 5e5fbb2..a00e137 100644 +--- a/Modules/_posixsubprocess.c ++++ b/Modules/_posixsubprocess.c +@@ -87,8 +87,8 @@ + + #define _posixsubprocessstate_global get_posixsubprocess_state(PyState_FindModule(&_posixsubprocessmodule)) + +-/* If gc was disabled, call gc.enable(). Return 0 on success. */ +-static int ++/* If gc was disabled, call gc.enable(). Ignore errors. */ ++static void + _enable_gc(int need_to_reenable_gc, PyObject *gc_module) + { + PyObject *result; +@@ -98,15 +98,17 @@ + PyErr_Fetch(&exctype, &val, &tb); + result = PyObject_CallMethodNoArgs( + gc_module, _posixsubprocessstate_global->enable); ++ if (result == NULL) { ++ /* We might have created a child process at this point, we ++ * we have no good way to handle a failure to reenable GC ++ * and return information about the child process. */ ++ PyErr_Print(); ++ } ++ Py_XDECREF(result); + if (exctype != NULL) { + PyErr_Restore(exctype, val, tb); + } +- if (result == NULL) { +- return 1; +- } +- Py_DECREF(result); + } +- return 0; + } + + +@@ -774,7 +776,7 @@ struct linux_dirent64 { + int child_umask; + PyObject *cwd_obj, *cwd_obj2 = NULL; + const char *cwd; +- pid_t pid; ++ pid_t pid = -1; + int need_to_reenable_gc = 0; + char *const *exec_array, *const *argv = NULL, *const *envp = NULL; + Py_ssize_t arg_num, num_groups = 0; +@@ -1010,8 +1012,6 @@ struct linux_dirent64 { + sigset_t all_sigs; + sigfillset(&all_sigs); + if ((saved_errno = pthread_sigmask(SIG_BLOCK, &all_sigs, &old_sigs))) { +- errno = saved_errno; +- PyErr_SetFromErrno(PyExc_OSError); + goto cleanup; + } + old_sigmask = &old_sigs; +@@ -1050,50 +1050,33 @@ struct linux_dirent64 { + } + #endif + +- Py_XDECREF(cwd_obj2); +- + if (need_after_fork) + PyOS_AfterFork_Parent(); +- if (envp) +- _Py_FreeCharPArray(envp); +- if (argv) +- _Py_FreeCharPArray(argv); +- _Py_FreeCharPArray(exec_array); +- +- /* Reenable gc in the parent process (or if fork failed). */ +- if (_enable_gc(need_to_reenable_gc, gc_module)) { +- pid = -1; +- } +- PyMem_RawFree(groups); +- Py_XDECREF(preexec_fn_args_tuple); +- Py_XDECREF(gc_module); + +- if (pid == -1) { ++cleanup: ++ if (saved_errno != 0) { + errno = saved_errno; + /* We can't call this above as PyOS_AfterFork_Parent() calls back + * into Python code which would see the unreturned error. */ + PyErr_SetFromErrno(PyExc_OSError); +- return NULL; /* fork() failed. */ + } + +- return PyLong_FromPid(pid); +- +-cleanup: ++ Py_XDECREF(preexec_fn_args_tuple); ++ PyMem_RawFree(groups); + Py_XDECREF(cwd_obj2); + if (envp) + _Py_FreeCharPArray(envp); ++ Py_XDECREF(converted_args); ++ Py_XDECREF(fast_args); + if (argv) + _Py_FreeCharPArray(argv); + if (exec_array) + _Py_FreeCharPArray(exec_array); + +- PyMem_RawFree(groups); +- Py_XDECREF(converted_args); +- Py_XDECREF(fast_args); +- Py_XDECREF(preexec_fn_args_tuple); + _enable_gc(need_to_reenable_gc, gc_module); + Py_XDECREF(gc_module); +- return NULL; ++ ++ return pid == -1 ? NULL : PyLong_FromPid(pid); + } + + +-- +1.8.3.1 + diff --git a/python3.spec b/python3.spec index ff3903174ffe6106d6bf15b5ca5fa5002f80f5bd..3e7fccd3e2d4fdd5054c961121e9e0d1967b55ad 100644 --- a/python3.spec +++ b/python3.spec @@ -3,7 +3,7 @@ Summary: Interpreter of the Python3 programming language URL: https://www.python.org/ Version: 3.9.9 -Release: 14 +Release: 15 License: Python-2.0 %global branchversion 3.9 @@ -94,6 +94,10 @@ Patch6002: backport-bpo-20369-concurrent.futures.wait-now-deduplicates-f.patch Patch6003: Make-mailcap-refuse-to-match-unsafe-filenam.patch Patch6004: backport-CVE-2021-28861.patch Patch6005: backport-CVE-2020-10735.patch +Patch6006: backport-bpo-35823-subprocess-Use-vfork-instead-of-fork-on-Li.patch +Patch6007: backport-bpo-35823-subprocess-Fix-handling-of-pthread_sigmask.patch +Patch6008: backport-bpo-35823-Allow-setsid-after-vfork-on-Linux.-GH-2294.patch +Patch6009: backport-bpo-42146-Unify-cleanup-in-subprocess_fork_exec-GH-2.patch Patch9000: add-the-sm3-method-for-obtaining-the-salt-value.patch @@ -185,6 +189,10 @@ rm -r Modules/expat %patch6003 -p1 %patch6004 -p1 %patch6005 -p1 +%patch6006 -p1 +%patch6007 -p1 +%patch6008 -p1 +%patch6009 -p1 %patch9000 -p1 @@ -801,6 +809,12 @@ export BEP_GTDLIST="$BEP_GTDLIST_TMP" %{_mandir}/*/* %changelog +* Thu Sep 22 zhuofeng - 3.9.9-15 +- Type:bugfix +- CVE:NA +- SUG:NA +- DESC:sync patches + * Thu Sep 08 shixuantong - 3.9.9-14 - Type:CVE - CVE:CVE-2020-10735