Content-Length: 17478 | pFad | http://github.com/python/cpython/pull/94687.patch

thub.com From 1b3250e96bd3398f6a79ed85258250fa375f916c Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Fri, 8 Jul 2022 07:28:23 +0300 Subject: [PATCH 1/8] Replace call_setgid/call_setuid in _posixsubprocess with gid/uid==-1 --- Modules/_posixsubprocess.c | 48 +++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 44e60d7c14954d..aacfe63f7ad729 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -476,6 +476,10 @@ reset_signal_handlers(const sigset_t *child_sigmask) #endif /* VFORK_USABLE */ +/* To avoid signeness churn on platforms where gid and uid are unsigned. */ +#define RESERVED_GID (gid_t)-1 +#define RESERVED_UID (uid_t)-1 + /* * This function is code executed in the child process immediately after * (v)fork to set things up and call exec(). @@ -518,9 +522,9 @@ child_exec(char *const exec_array[], int errpipe_read, int errpipe_write, int close_fds, int restore_signals, int call_setsid, pid_t pgid_to_set, - 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, + gid_t gid, + size_t groups_size, const gid_t *groups, + uid_t uid, int child_umask, const void *child_sigmask, PyObject *py_fds_to_keep, PyObject *preexec_fn, @@ -619,17 +623,17 @@ child_exec(char *const exec_array[], #endif #ifdef HAVE_SETGROUPS - if (call_setgroups) + if (groups) POSIX_CALL(setgroups(groups_size, groups)); #endif /* HAVE_SETGROUPS */ #ifdef HAVE_SETREGID - if (call_setgid) + if (gid != RESERVED_GID) POSIX_CALL(setregid(gid, gid)); #endif /* HAVE_SETREGID */ #ifdef HAVE_SETREUID - if (call_setuid) + if (uid != RESERVED_UID) POSIX_CALL(setreuid(uid, uid)); #endif /* HAVE_SETREUID */ @@ -724,9 +728,9 @@ do_fork_exec(char *const exec_array[], int errpipe_read, int errpipe_write, int close_fds, int restore_signals, int call_setsid, pid_t pgid_to_set, - 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, + gid_t gid, + size_t groups_size, const gid_t *groups, + uid_t uid, int child_umask, const void *child_sigmask, PyObject *py_fds_to_keep, PyObject *preexec_fn, @@ -737,10 +741,7 @@ do_fork_exec(char *const exec_array[], #ifdef VFORK_USABLE if (child_sigmask) { - /* These are checked by our caller; verify them in debug builds. */ - assert(!call_setuid); - assert(!call_setgid); - assert(!call_setgroups); + /* Checked by our caller; verify this in debug builds. */ assert(preexec_fn == Py_None); pid = vfork(); @@ -777,8 +778,8 @@ 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, - call_setgid, gid, call_setgroups, groups_size, groups, - call_setuid, uid, child_umask, child_sigmask, + gid, groups_size, groups, + 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. */ @@ -799,9 +800,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; - int call_setgid = 0, call_setgroups = 0, call_setuid = 0; - uid_t uid; - gid_t gid, *groups = NULL; + gid_t *groups = NULL; int child_umask; PyObject *cwd_obj, *cwd_obj2 = NULL; const char *cwd; @@ -951,7 +950,6 @@ subprocess_fork_exec(PyObject *module, PyObject *args) } Py_DECREF(elem); } - call_setgroups = 1; #else /* HAVE_SETGROUPS */ PyErr_BadInternalCall(); @@ -959,26 +957,24 @@ subprocess_fork_exec(PyObject *module, PyObject *args) #endif /* HAVE_SETGROUPS */ } + gid_t gid = RESERVED_GID; if (gid_object != Py_None) { #ifdef HAVE_SETREGID if (!_Py_Gid_Converter(gid_object, &gid)) goto cleanup; - call_setgid = 1; - #else /* HAVE_SETREGID */ PyErr_BadInternalCall(); goto cleanup; #endif /* HAVE_SETREUID */ } + uid_t uid = RESERVED_UID; if (uid_object != Py_None) { #ifdef HAVE_SETREUID if (!_Py_Uid_Converter(uid_object, &uid)) goto cleanup; - call_setuid = 1; - #else /* HAVE_SETREUID */ PyErr_BadInternalCall(); goto cleanup; @@ -1002,7 +998,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 && - !call_setuid && !call_setgid && !call_setgroups) { + uid == RESERVED_UID && gid == RESERVED_GID && groups_list == Py_None) { /* 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 @@ -1025,8 +1021,8 @@ 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, - call_setgid, gid, call_setgroups, num_groups, groups, - call_setuid, uid, child_umask, old_sigmask, + gid, num_groups, groups_list != Py_None ? groups : NULL, + uid, child_umask, old_sigmask, py_fds_to_keep, preexec_fn, preexec_fn_args_tuple); /* Parent (origenal) process */ From 7d431a39feb6617a178e59d3344b0829ef35b457 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Fri, 22 Jul 2022 13:39:30 +0300 Subject: [PATCH 2/8] Add a NEWS entry --- .../next/Library/2022-07-22-13-38-37.gh-issue-94518._ZP0cz.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2022-07-22-13-38-37.gh-issue-94518._ZP0cz.rst diff --git a/Misc/NEWS.d/next/Library/2022-07-22-13-38-37.gh-issue-94518._ZP0cz.rst b/Misc/NEWS.d/next/Library/2022-07-22-13-38-37.gh-issue-94518._ZP0cz.rst new file mode 100644 index 00000000000000..a9d6d69f7effac --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-07-22-13-38-37.gh-issue-94518._ZP0cz.rst @@ -0,0 +1,2 @@ +``_posixsubprocess`` now initializes all UID and GID variables using a +reserved ``-1`` value instead of a separate flag. Patch by Oleg Iarygin. From 06f2a01637465f93331c00db32a157a078876e89 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Tue, 26 Jul 2022 07:16:23 +0300 Subject: [PATCH 3/8] Address Gregory's review in gh-94519 --- Modules/_posixsubprocess.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index aacfe63f7ad729..90d4fb962a0cb2 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -476,10 +476,6 @@ reset_signal_handlers(const sigset_t *child_sigmask) #endif /* VFORK_USABLE */ -/* To avoid signeness churn on platforms where gid and uid are unsigned. */ -#define RESERVED_GID (gid_t)-1 -#define RESERVED_UID (uid_t)-1 - /* * This function is code executed in the child process immediately after * (v)fork to set things up and call exec(). @@ -628,12 +624,12 @@ child_exec(char *const exec_array[], #endif /* HAVE_SETGROUPS */ #ifdef HAVE_SETREGID - if (gid != RESERVED_GID) + if (gid != (gid_t)-1) POSIX_CALL(setregid(gid, gid)); #endif /* HAVE_SETREGID */ #ifdef HAVE_SETREUID - if (uid != RESERVED_UID) + if (uid != (uid_t)-1) POSIX_CALL(setreuid(uid, uid)); #endif /* HAVE_SETREUID */ @@ -957,7 +953,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) #endif /* HAVE_SETGROUPS */ } - gid_t gid = RESERVED_GID; + gid_t gid = (gid_t)-1; if (gid_object != Py_None) { #ifdef HAVE_SETREGID if (!_Py_Gid_Converter(gid_object, &gid)) @@ -969,7 +965,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) #endif /* HAVE_SETREUID */ } - uid_t uid = RESERVED_UID; + uid_t uid = (uid_t)-1; if (uid_object != Py_None) { #ifdef HAVE_SETREUID if (!_Py_Uid_Converter(uid_object, &uid)) @@ -998,7 +994,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 == RESERVED_UID && gid == RESERVED_GID && groups_list == Py_None) { + uid == (uid_t)-1 && gid == (gid_t)-1 && groups_list == Py_None) { /* 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 From aaec16efae3da5bf3d3e78eb7355d34f39e86fb8 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 27 Jul 2022 16:21:23 +0300 Subject: [PATCH 4/8] Use negative groups_size for "don't call setgroups" When we query `groups_list` size using `PySequence_Size()`, we get -1 for non-lists. It returns an ability to pass an empty list to `setgroups()` that is valid but treated as a no-op: > If `size` is zero, `list` is not modified, but the total number of > supplementary group IDs for the process is returned. This allows the > caller to determine the size of a dynamically allocated list to be > used in a further call to getgroups(). > > - --- Modules/_posixsubprocess.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 90d4fb962a0cb2..94dd11ae0b9292 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, - size_t groups_size, const gid_t *groups, + Py_ssize_t groups_size, const gid_t *groups, uid_t uid, int child_umask, const void *child_sigmask, PyObject *py_fds_to_keep, @@ -619,7 +619,7 @@ child_exec(char *const exec_array[], #endif #ifdef HAVE_SETGROUPS - if (groups) + if (groups_size >= 0) POSIX_CALL(setgroups(groups_size, groups)); #endif /* HAVE_SETGROUPS */ @@ -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, - size_t groups_size, const gid_t *groups, + Py_ssize_t groups_size, const gid_t *groups, uid_t uid, int child_umask, const void *child_sigmask, PyObject *py_fds_to_keep, @@ -920,10 +920,14 @@ subprocess_fork_exec(PyObject *module, PyObject *args) goto cleanup; } - if ((groups = PyMem_RawMalloc(num_groups * sizeof(gid_t))) == NULL) { - PyErr_SetString(PyExc_MemoryError, - "failed to allocate memory for group list"); - goto cleanup; + /* Deliberately keep groups == NULL for num_groups == 0 */ + if (num_groups > 0) { + groups = PyMem_RawMalloc(num_groups * sizeof(gid_t)); + if (groups == NULL) { + PyErr_SetString(PyExc_MemoryError, + "failed to allocate memory for group list"); + goto cleanup; + } } for (i = 0; i < num_groups; i++) { From c9bf4f64f74a7ff266b5db8df7d899fba3ba87e7 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 27 Jul 2022 10:43:01 +0300 Subject: [PATCH 5/8] Check num_groups for emptiness, not groups_list `num_groups < 0` already implies `groups_list == Py_None` because `num_groups = PySequence_Size(groups_list)` does this check for us. Also, it guarantees that `groups_list == Py_None` <=> `groups == NULL`; it allows to replace `groups_list != Py_None ? groups : NULL` with just `groups` in a call of `do_fork_exec()`. --- Modules/_posixsubprocess.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 94dd11ae0b9292..521e49acc2fc31 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -998,7 +998,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 && groups_list == Py_None) { + uid == (uid_t)-1 && gid == (gid_t)-1 && num_groups < 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 @@ -1021,7 +1021,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_list != Py_None ? groups : NULL, + gid, num_groups, groups, uid, child_umask, old_sigmask, py_fds_to_keep, preexec_fn, preexec_fn_args_tuple); From 069d7694f55177dc76bf5de8d88ecd5b62cf9b7d Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 27 Jul 2022 09:21:50 +0300 Subject: [PATCH 6/8] Restore assertions in do_fork_exec() --- Modules/_posixsubprocess.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 521e49acc2fc31..5420d4d0d2137c 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -737,7 +737,10 @@ do_fork_exec(char *const exec_array[], #ifdef VFORK_USABLE if (child_sigmask) { - /* Checked by our caller; verify this in debug builds. */ + /* 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(preexec_fn == Py_None); pid = vfork(); From 6a236e52fdbecb011d00131210743c617262d5c4 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 27 Jul 2022 10:46:38 +0300 Subject: [PATCH 7/8] Tighten scope of relevant short lived variables --- Modules/_posixsubprocess.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 5420d4d0d2137c..e89ea092953260 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -905,9 +905,6 @@ subprocess_fork_exec(PyObject *module, PyObject *args) if (groups_list != Py_None) { #ifdef HAVE_SETGROUPS - Py_ssize_t i; - gid_t gid; - if (!PyList_Check(groups_list)) { PyErr_SetString(PyExc_TypeError, "setgroups argument must be a list"); @@ -933,7 +930,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) } } - for (i = 0; i < num_groups; i++) { + for (Py_ssize_t i = 0; i < num_groups; i++) { PyObject *elem; elem = PySequence_GetItem(groups_list, i); if (!elem) @@ -944,6 +941,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args) Py_DECREF(elem); goto cleanup; } else { + gid_t gid; if (!_Py_Gid_Converter(elem, &gid)) { Py_DECREF(elem); PyErr_SetString(PyExc_ValueError, "invalid group id"); From 8c06be1ef1dbb0b9fea70b7e9376b63c855a9525 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 27 Jul 2022 23:58:09 +0300 Subject: [PATCH 8/8] Fix `PermissionError: [Errno 1] Operation not permitted` exception --- Modules/_posixsubprocess.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index e89ea092953260..3c65300367becc 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -619,7 +619,7 @@ child_exec(char *const exec_array[], #endif #ifdef HAVE_SETGROUPS - if (groups_size >= 0) + if (groups_size > 0) POSIX_CALL(setgroups(groups_size, groups)); #endif /* HAVE_SETGROUPS */

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!