8000 gh-94518: [_posixsubprocess] Replace variable validity flags with res… · python/cpython@124af17 · GitHub
[go: up one dir, main page]
Skip to content

Commit 124af17

Browse files
arhadthedevarhadthedev
authored
gh-94518: [_posixsubprocess] Replace variable validity flags with reserved values (#94687)
Have _posixsubprocess.c stop using boolean flags to say if gid and uid values were supplied and action is required. Such an implicit "either initialized or look somewhere else" confused both the reader (another mental connection to constantly track between functions) and a compiler (warnings on potentially uninitialized variables being passed). Instead, we can utilize a special group/user id as a flag value -1 defined by POSIX but used nowhere else. Namely: gid: call_setgid = False → gid = -1 uid: call_setuid = False → uid = -1 groups: call_setgroups = False → groups = NULL (obtained with (groups_list != Py_None) ? groups : NULL) This PR is required for #94519.
1 parent 49cae39 commit 124af17

File tree

2 files changed

+32
-33
lines changed

2 files changed

+32
-33
lines changed

‎Misc/NEWS.d/next/Library/2022-07-22-13-38-37.gh-issue-94518._ZP0cz.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
``_posixsubprocess`` now initializes all UID and GID variables using a
2+
reserved ``-1`` value instead of a separate flag. Patch by Oleg Iarygin.

‎Modules/_posixsubprocess.c‎

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -518,9 +518,9 @@ child_exec(char *const exec_array[],
518518
int errpipe_read, int errpipe_write,
519519
int close_fds, int restore_signals,
520520
int call_setsid, pid_t pgid_to_set,
521-
int call_setgid, gid_t gid,
522-
int call_setgroups, size_t groups_size, const gid_t *groups,
523-
int call_setuid, uid_t uid, int child_umask,
521+
gid_t gid,
522+
Py_ssize_t groups_size, const gid_t *groups,
523+
uid_t uid, int child_umask,
524524
const void *child_sigmask,
525525
PyObject *py_fds_to_keep,
526526
PyObject *preexec_fn,
@@ -619,17 +619,17 @@ child_exec(char *const exec_array[],
619619
#endif
620620

621621
#ifdef HAVE_SETGROUPS
622-
if (call_setgroups)
622+
if (groups_size > 0)
623623
POSIX_CALL(setgroups(groups_size, groups));
624624
#endif /* HAVE_SETGROUPS */
625625

626626
#ifdef HAVE_SETREGID
627-
if (call_setgid)
627+
if (gid != (gid_t)-1)
628628
POSIX_CALL(setregid(gid, gid));
629629
#endif /* HAVE_SETREGID */
630630

631631
#ifdef HAVE_SETREUID
632-
if (call_setuid)
632+
if (uid != (uid_t)-1)
633633
POSIX_CALL(setreuid(uid, uid));
634634
#endif /* HAVE_SETREUID */
635635

@@ -724,9 +724,9 @@ do_fork_exec(char *const exec_array[],
724724
int errpipe_read, int errpipe_write,
725725
int close_fds, int restore_signals,
726726
int call_setsid, pid_t pgid_to_set,
727-
int call_setgid, gid_t gid,
728-
int call_setgroups, size_t groups_size, const gid_t *groups,
729-
int call_setuid, uid_t uid, int child_umask,
727+
gid_t gid,
728+
Py_ssize_t groups_size, const gid_t *groups,
729+
uid_t uid, int child_umask,
730730
const void *child_sigmask,
731731
PyObject *py_fds_to_keep,
732732
PyObject *preexec_fn,
@@ -738,9 +738,9 @@ do_fork_exec(char *const exec_array[],
738738
#ifdef VFORK_USABLE
739739
if (child_sigmask) {
740740
/* These are checked by our caller; verify them in debug builds. */
741-
assert(!call_setuid);
742-
assert(!call_setgid);
743-
assert(!call_setgroups);
741+
assert(uid == (uid_t)-1);
742+
assert(gid == (gid_t)-1);
743+
assert(groups_size < 0);
744744
assert(preexec_fn == Py_None);
745745

746746
pid = vfork();
@@ -777,8 +777,8 @@ do_fork_exec(char *const exec_array[],
777777
p2cread, p2cwrite, c2pread, c2pwrite,
778778
errread, errwrite, errpipe_read, errpipe_write,
779779
close_fds, restore_signals, call_setsid, pgid_to_set,
780-
call_setgid, gid, call_setgroups, groups_size, groups,
781-
call_setuid, uid, child_umask, child_sigmask,
780+
gid, groups_size, groups,
781+
uid, child_umask, child_sigmask,
782782
py_fds_to_keep, preexec_fn, preexec_fn_args_tuple);
783783
_exit(255);
784784
return 0; /* Dead code to avoid a potential compiler warning. */
@@ -799,9 +799,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
799799
int errpipe_read, errpipe_write, close_fds, restore_signals;
800800
int call_setsid;
801801
pid_t pgid_to_set = -1;
802-
int call_setgid = 0, call_setgroups = 0, call_setuid = 0;
803-
uid_t uid;
804-
gid_t gid, *groups = NULL;
802+
gid_t *groups = NULL;
805803
int child_umask;
806804
PyObject *cwd_obj, *cwd_obj2 = NULL;
807805
const char *cwd;
@@ -899,9 +897,6 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
899897

900898
if (groups_list != Py_None) {
901899
#ifdef HAVE_SETGROUPS
902-
Py_ssize_t i;
903-
gid_t gid;
904-
905900
if (!PyList_Check(groups_list)) {
906901
PyErr_SetString(PyExc_TypeError,
907902
"setgroups argument must be a list");
@@ -917,13 +912,17 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
917912
goto cleanup;
918913
}
919914

920-
if ((groups = PyMem_RawMalloc(num_groups * sizeof(gid_t))) == NULL) {
921-
PyErr_SetString(PyExc_MemoryError,
922-
"failed to allocate memory for group list");
923-
goto cleanup;
915+
/* Deliberately keep groups == NULL for num_groups == 0 */
916+
if (num_groups > 0) {
917+
groups = PyMem_RawMalloc(num_groups * sizeof(gid_t));
918+
if (groups == NULL) {
919+
PyErr_SetString(PyExc_MemoryError,
920+
"failed to allocate memory for group list");
921+
goto cleanup;
922+
}
924923
}
925924

926-
for (i = 0; i < num_groups; i++) {
925+
for (Py_ssize_t i = 0; i < num_groups; i++) {
927926
PyObject *elem;
928927
elem = PySequence_GetItem(groups_list, i);
929928
if (!elem)
@@ -934,6 +933,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
934933
Py_DECREF(elem);
935934
goto cleanup;
936935
} else {
936+
gid_t gid;
937937
if (!_Py_Gid_Converter(elem, &gid)) {
938938
Py_DECREF(elem);
939939
PyErr_SetString(PyExc_ValueError, "invalid group id");
@@ -943,34 +943,31 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
943943
}
944944
Py_DECREF(elem);
945945
}
946-
call_setgroups = 1;
947946

948947
#else /* HAVE_SETGROUPS */
949948
PyErr_BadInternalCall();
950949
goto cleanup;
951950
#endif /* HAVE_SETGROUPS */
952951
}
953952

953+
gid_t gid = (gid_t)-1;
954954
if (gid_object != Py_None) {
955955
#ifdef HAVE_SETREGID
956956
if (!_Py_Gid_Converter(gid_object, &gid))
957957
goto cleanup;
958958

959-
call_setgid = 1;
960-
961959
#else /* HAVE_SETREGID */
962960
PyErr_BadInternalCall();
963961
goto cleanup;
964962
#endif /* HAVE_SETREUID */
965963
}
966964

965+
uid_t uid = (uid_t)-1;
967966
if (uid_object != Py_None) {
968967
#ifdef HAVE_SETREUID
969968
if (!_Py_Uid_Converter(uid_object, &uid))
970969
goto cleanup;
971970

972-
call_setuid = 1;
973-
974971
#else /* HAVE_SETREUID */
975972
PyErr_BadInternalCall();
976973
goto cleanup;
@@ -994,7 +991,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
994991
/* Use vfork() only if it's safe. See the comment above child_exec(). */
995992
sigset_t old_sigs;
996993
if (preexec_fn == Py_None && allow_vfork &&
997-
!call_setuid && !call_setgid && !call_setgroups) {
994+
uid == (uid_t)-1 && gid == (gid_t)-1 && num_groups < 0) {
998995
/* Block all signals to ensure that no signal handlers are run in the
999996
* child process while it shares memory with us. Note that signals
1000997
* used internally by C libraries won't be blocked by
@@ -1017,8 +1014,8 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
10171014
p2cread, p2cwrite, c2pread, c2pwrite,
10181015
errread, errwrite, errpipe_read, errpipe_write,
10191016
close_fds, restore_signals, call_setsid, pgid_to_set,
1020-
call_setgid, gid, call_setgroups, num_groups, groups,
1021-
call_setuid, uid, child_umask, old_sigmask,
1017+
gid, num_groups, groups,
1018+
uid, child_umask, old_sigmask,
10221019
py_fds_to_keep, preexec_fn, preexec_fn_args_tuple);
10231020

10241021
/* Parent (original) process */

0 commit comments

Comments
 (0)
0