mirrors/cpython
1
0
Fork 0
mirror of https://github.com/python/cpython.git synced 2025-08-09 19:38:42 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 10

[3.12] gh-112334: Restore subprocess's use of vfork() & fix extra_groups=[] behavior (GH-112617) (#112731)

Browse source 
Restore `subprocess`'s intended use of `vfork()` by default for performance on Linux;
also fixes the behavior of `extra_groups=[]` which was unintentionally broken in 3.12.0:

Fixed a performance regression in 3.12's :mod:`subprocess` on Linux where it
would no longer use the fast-path ``vfork()`` system call when it could have
due to a logic bug, instead falling back to the safe but slower ``fork()``.

Also fixed a security bug introduced in 3.12.0.  If a value of ``extra_groups=[]``
was passed to :mod:`subprocess.Popen` or related APIs, the underlying
``setgroups(0, NULL)`` system call to clear the groups list would not be made
in the child process prior to ``exec()``.

The security issue was identified via code inspection in the process of
fixing the first bug.  Thanks to @vain for the detailed report and
analysis in the initial bug on Github.

(cherry picked from commit 9fe7655c6c)

+ Reword NEWS for the bugfix/security release. (mentions the assigned CVE number)

Co-authored-by: Gregory P. Smith [Google LLC] <greg@krypto.org>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
This commit is contained in:
Miss Islington (bot) 2023-12-05 00:33:59 +01:00 • committed by GitHub
parent 494cd508c0
commit 85bbfa8a4b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 37 additions and 23 deletions
Download patch file Download diff file Expand all files Collapse all files

12
Modules/_posixsubprocess.c
View file

@ -682,8 +682,10 @@ child_exec(char *const exec_array[],
#endif
#ifdef HAVE_SETGROUPS
if (extra_group_size > 0)
if (extra_group_size >= 0) {
assert((extra_group_size == 0) == (extra_groups == NULL));
POSIX_CALL(setgroups(extra_group_size, extra_groups));
}
#endif /* HAVE_SETGROUPS */
#ifdef HAVE_SETREGID
@ -937,7 +939,6 @@ subprocess_fork_exec_impl(PyObject *module, PyObject *process_args,
pid_t pid = -1;
int need_to_reenable_gc = 0;
char *const *argv = NULL, *const *envp = NULL;
Py_ssize_t extra_group_size = 0;
int need_after_fork = 0;
int saved_errno = 0;
int *c_fds_to_keep = NULL;
@ -1018,6 +1019,13 @@ subprocess_fork_exec_impl(PyObject *module, PyObject *process_args,
cwd = PyBytes_AsString(cwd_obj2);
}
// Special initial value meaning that subprocess API was called with
// extra_groups=None leading to _posixsubprocess.fork_exec(gids=None).
// We use this to differentiate between code desiring a setgroups(0, NULL)
// call vs no call at all. The fast vfork() code path could be used when
// there is no setgroups call.
Py_ssize_t extra_group_size = -2;
if (extra_groups_packed != Py_None) {
#ifdef HAVE_SETGROUPS
if (!PyList_Check(extra_groups_packed)) {