-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
bpo-42655: Fix subprocess extra_groups gid conversion #23762
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-42655: Fix subprocess extra_groups gid conversion #23762
Conversation
Please file an issue for tracking, thanks!
… |
This looks good. i'm not sure why void* was ever used on those APIs, that doesn't make much sense to me. i'm not aware of any platforms without uid_t and gid_t. Can you add a news entry using blurb or the blurb_it service? |
Yes, and
Done |
Because it is the signature of the converter function for "O&" format unit in PyArg_Parse. And many other converters have |
Thanks @kulikjak for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9. |
(cherry picked from commit 0159e5e) Co-authored-by: Jakub Kulík <[email protected]>
GH-23997 is a backport of this pull request to the 3.9 branch. |
(cherry picked from commit 0159e5e) Co-authored-by: Jakub Kulík <[email protected]>
C function
subprocess_fork_exec
incorrectly transforms gids fromextra_groups
argument because it passesunsigned long*
rather thanpid_t*
into the_Py_Gid_Converter()
. Assuming thatgid_t
is 32 bit andunsigned long
is 64 bit (which it often is),*(gid_t *)p = gid;
then incorrectly overwrites only part of that variable, leaving the other one filled with previous garbage.I found this on Solaris, but I am pretty sure that this doesn't work correctly on Linux as well, since both use
unsigned int
asgid_t
.I am not sure whether a small fix like this one needs an issue, but if so, I am happy to create one.
https://bugs.python.org/issue42655