From 73ce627144cdcb857638ee69035ede2d12d585ba Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 15 Jan 2023 12:34:40 +0400 Subject: [PATCH 1/7] Rename `group*` to `extra_group*` to avoid confusion --- Modules/_posixsubprocess.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 516f11d3543d58..f7dc88bcfa5948 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -519,7 +519,7 @@ child_exec(char *const exec_array[], int close_fds, int restore_signals, int call_setsid, pid_t pgid_to_set, gid_t gid, - Py_ssize_t groups_size, const gid_t *groups, + Py_ssize_t extra_group_size, const gid_t *extra_groups, uid_t uid, int child_umask, const void *child_sigmask, PyObject *py_fds_to_keep, @@ -619,8 +619,8 @@ child_exec(char *const exec_array[], #endif #ifdef HAVE_SETGROUPS - if (groups_size > 0) - POSIX_CALL(setgroups(groups_size, groups)); + if (extra_group_size > 0) + POSIX_CALL(setgroups(extra_group_size, extra_groups)); #endif /* HAVE_SETGROUPS */ #ifdef HAVE_SETREGID @@ -725,7 +725,7 @@ do_fork_exec(char *const exec_array[], int close_fds, int restore_signals, int call_setsid, pid_t pgid_to_set, gid_t gid, - Py_ssize_t groups_size, const gid_t *groups, + Py_ssize_t extra_group_size, const gid_t *extra_groups, uid_t uid, int child_umask, const void *child_sigmask, PyObject *py_fds_to_keep, @@ -740,7 +740,7 @@ do_fork_exec(char *const exec_array[], /* These are checked by our caller; verify them in debug builds. */ assert(uid == (uid_t)-1); assert(gid == (gid_t)-1); - assert(groups_size < 0); + assert(extra_group_size < 0); assert(preexec_fn == Py_None); pid = vfork(); @@ -777,7 +777,7 @@ do_fork_exec(char *const exec_array[], p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite, errpipe_read, errpipe_write, close_fds, restore_signals, call_setsid, pgid_to_set, - gid, groups_size, groups, + gid, extra_group_size, extra_groups, uid, child_umask, child_sigmask, py_fds_to_keep, preexec_fn, preexec_fn_args_tuple); _exit(255); @@ -799,7 +799,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) int errpipe_read, errpipe_write, close_fds, restore_signals; int call_setsid; pid_t pgid_to_set = -1; - gid_t *groups = NULL; + gid_t *extra_groups = NULL; int child_umask; PyObject *cwd_obj, *cwd_obj2 = NULL; const char *cwd; @@ -908,14 +908,14 @@ subprocess_fork_exec(PyObject *module, PyObject *args) goto cleanup; if (num_groups > MAX_GROUPS) { - PyErr_SetString(PyExc_ValueError, "too many groups"); + PyErr_SetString(PyExc_ValueError, "too many extra_groups"); goto cleanup; } - /* Deliberately keep groups == NULL for num_groups == 0 */ + /* Deliberately keep extra_groups == NULL for num_groups == 0 */ if (num_groups > 0) { - groups = PyMem_RawMalloc(num_groups * sizeof(gid_t)); - if (groups == NULL) { + extra_groups = PyMem_RawMalloc(num_groups * sizeof(gid_t)); + if (extra_groups == NULL) { PyErr_SetString(PyExc_MemoryError, "failed to allocate memory for group list"); goto cleanup; @@ -929,7 +929,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) goto cleanup; if (!PyLong_Check(elem)) { PyErr_SetString(PyExc_TypeError, - "groups must be integers"); + "extra_groups must be integers"); Py_DECREF(elem); goto cleanup; } else { @@ -939,7 +939,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) PyErr_SetString(PyExc_ValueError, "invalid group id"); goto cleanup; } - groups[i] = gid; + extra_groups[i] = gid; } Py_DECREF(elem); } @@ -1014,7 +1014,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite, errpipe_read, errpipe_write, close_fds, restore_signals, call_setsid, pgid_to_set, - gid, num_groups, groups, + gid, num_groups, extra_groups, uid, child_umask, old_sigmask, py_fds_to_keep, preexec_fn, preexec_fn_args_tuple); @@ -1054,7 +1054,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) } Py_XDECREF(preexec_fn_args_tuple); - PyMem_RawFree(groups); + PyMem_RawFree(extra_groups); Py_XDECREF(cwd_obj2); if (envp) _Py_FreeCharPArray(envp); From c40e9404f8be730e2d1eb6f0a2357970f43fe8ae Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 15 Jan 2023 09:11:31 +0000 Subject: [PATCH 2/7] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2023-01-15-09-11-30.gh-issue-94518.jvxtxm.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2023-01-15-09-11-30.gh-issue-94518.jvxtxm.rst diff --git a/Misc/NEWS.d/next/Library/2023-01-15-09-11-30.gh-issue-94518.jvxtxm.rst b/Misc/NEWS.d/next/Library/2023-01-15-09-11-30.gh-issue-94518.jvxtxm.rst new file mode 100644 index 00000000000000..57fb05461dcfb7 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-01-15-09-11-30.gh-issue-94518.jvxtxm.rst @@ -0,0 +1 @@ +``groups_size`` and ``groups`` variables of ``_posixsubprocess`` module are renamed into ``extra_group_size`` and ``extra_group`` to stress that they add group affinity, not replace it. Patch by Oleg Iarygin. From 0b308b28e09acfcf48543f46ffc28e806dae575f Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 15 Jan 2023 13:42:52 +0400 Subject: [PATCH 3/7] Rename `num_groups` into `extra_group_size` --- Modules/_posixsubprocess.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index f7dc88bcfa5948..ad3d0342a15a17 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -806,7 +806,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) 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; + Py_ssize_t arg_num, extra_group_size = 0; int need_after_fork = 0; int saved_errno = 0; int allow_vfork; @@ -902,19 +902,19 @@ subprocess_fork_exec(PyObject *module, PyObject *args) "setgroups argument must be a list"); goto cleanup; } - num_groups = PySequence_Size(groups_list); + extra_group_size = PySequence_Size(groups_list); - if (num_groups < 0) + if (extra_group_size < 0) goto cleanup; - if (num_groups > MAX_GROUPS) { + if (extra_group_size > MAX_GROUPS) { PyErr_SetString(PyExc_ValueError, "too many extra_groups"); goto cleanup; } - /* Deliberately keep extra_groups == NULL for num_groups == 0 */ - if (num_groups > 0) { - extra_groups = PyMem_RawMalloc(num_groups * sizeof(gid_t)); + /* Deliberately keep extra_groups == NULL for extra_group_size == 0 */ + if (extra_group_size > 0) { + extra_groups = PyMem_RawMalloc(extra_group_size * sizeof(gid_t)); if (extra_groups == NULL) { PyErr_SetString(PyExc_MemoryError, "failed to allocate memory for group list"); @@ -922,7 +922,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) } } - for (Py_ssize_t i = 0; i < num_groups; i++) { + for (Py_ssize_t i = 0; i < extra_group_size; i++) { PyObject *elem; elem = PySequence_GetItem(groups_list, i); if (!elem) @@ -991,7 +991,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) /* Use vfork() only if it's safe. See the comment above child_exec(). */ sigset_t old_sigs; if (preexec_fn == Py_None && allow_vfork && - uid == (uid_t)-1 && gid == (gid_t)-1 && num_groups < 0) { + uid == (uid_t)-1 && gid == (gid_t)-1 && extra_group_size < 0) { /* 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 @@ -1014,7 +1014,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite, errpipe_read, errpipe_write, close_fds, restore_signals, call_setsid, pgid_to_set, - gid, num_groups, extra_groups, + gid, extra_group_size, extra_groups, uid, child_umask, old_sigmask, py_fds_to_keep, preexec_fn, preexec_fn_args_tuple); From 1b10ce0e7d6df2ed265d0b2f4d2b02cff0c5bf4a Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 15 Jan 2023 13:43:07 +0400 Subject: [PATCH 4/7] Rename `groups_list` to `extra_groups_packed` --- Modules/_posixsubprocess.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index ad3d0342a15a17..fb4ca77e20c76b 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -793,7 +793,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) PyObject *env_list, *preexec_fn; PyObject *process_args, *converted_args = NULL, *fast_args = NULL; PyObject *preexec_fn_args_tuple = NULL; - PyObject *groups_list; + PyObject *extra_groups_packed; PyObject *uid_object, *gid_object; int p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite; int errpipe_read, errpipe_write, close_fds, restore_signals; @@ -819,7 +819,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) &p2cread, &p2cwrite, &c2pread, &c2pwrite, &errread, &errwrite, &errpipe_read, &errpipe_write, &restore_signals, &call_setsid, &pgid_to_set, - &gid_object, &groups_list, &uid_object, &child_umask, + &gid_object, &extra_groups_packed, &uid_object, &child_umask, &preexec_fn, &allow_vfork)) return NULL; @@ -895,14 +895,14 @@ subprocess_fork_exec(PyObject *module, PyObject *args) cwd = NULL; } - if (groups_list != Py_None) { + if (extra_groups_packed != Py_None) { #ifdef HAVE_SETGROUPS - if (!PyList_Check(groups_list)) { + if (!PyList_Check(extra_groups_packed)) { PyErr_SetString(PyExc_TypeError, "setgroups argument must be a list"); goto cleanup; } - extra_group_size = PySequence_Size(groups_list); + extra_group_size = PySequence_Size(extra_groups_packed); if (extra_group_size < 0) goto cleanup; @@ -924,7 +924,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) for (Py_ssize_t i = 0; i < extra_group_size; i++) { PyObject *elem; - elem = PySequence_GetItem(groups_list, i); + elem = PySequence_GetItem(extra_groups_packed, i); if (!elem) goto cleanup; if (!PyLong_Check(elem)) { @@ -1079,7 +1079,7 @@ PyDoc_STRVAR(subprocess_fork_exec_doc, p2cread, p2cwrite, c2pread, c2pwrite,\n\ errread, errwrite, errpipe_read, errpipe_write,\n\ restore_signals, call_setsid, pgid_to_set,\n\ - gid, groups_list, uid,\n\ + gid, extra_groups_packed, uid,\n\ preexec_fn)\n\ \n\ Forks a child process, closes parent file descriptors as appropriate in the\n\ From 4dfc19a8c3051c6f0697931818489a4fe27dc2b7 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 15 Jan 2023 13:46:55 +0400 Subject: [PATCH 5/7] Expand a news entry --- .../Library/2023-01-15-09-11-30.gh-issue-94518.jvxtxm.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2023-01-15-09-11-30.gh-issue-94518.jvxtxm.rst b/Misc/NEWS.d/next/Library/2023-01-15-09-11-30.gh-issue-94518.jvxtxm.rst index 57fb05461dcfb7..77563090464dbc 100644 --- a/Misc/NEWS.d/next/Library/2023-01-15-09-11-30.gh-issue-94518.jvxtxm.rst +++ b/Misc/NEWS.d/next/Library/2023-01-15-09-11-30.gh-issue-94518.jvxtxm.rst @@ -1 +1,3 @@ -``groups_size`` and ``groups`` variables of ``_posixsubprocess`` module are renamed into ``extra_group_size`` and ``extra_group`` to stress that they add group affinity, not replace it. Patch by Oleg Iarygin. +Group-related variables of ``_posixsubprocess`` module are renamed to +stress that supplimentary group affinity is added to a fork, not +replace the inherited ones. Patch by Oleg Iarygin. From 6f99a34c5abca33cd76c707c1d8888d872c1e6b4 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 15 Jan 2023 14:54:13 +0400 Subject: [PATCH 6/7] A dummy commit to rerun tests From b024647136b33ddd7326259280c9ff4a5f2932bb Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 21 Jan 2023 09:25:25 +0300 Subject: [PATCH 7/7] Return `groups_list` is a Python signature --- Modules/_posixsubprocess.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index fb4ca77e20c76b..f3ff39215eab76 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -1079,7 +1079,7 @@ PyDoc_STRVAR(subprocess_fork_exec_doc, p2cread, p2cwrite, c2pread, c2pwrite,\n\ errread, errwrite, errpipe_read, errpipe_write,\n\ restore_signals, call_setsid, pgid_to_set,\n\ - gid, extra_groups_packed, uid,\n\ + gid, extra_groups, uid,\n\ preexec_fn)\n\ \n\ Forks a child process, closes parent file descriptors as appropriate in the\n\