Skip to content

Conversation

@kulikjak
Copy link
Contributor

@kulikjak kulikjak commented Dec 14, 2020

C function subprocess_fork_exec incorrectly transforms gids from extra_groups argument because it passes unsigned long* rather than pid_t* into the _Py_Gid_Converter(). Assuming that gid_t is 32 bit and unsigned 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 as gid_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

@gpshead
Copy link
Member

gpshead commented Dec 14, 2020 via email

@kulikjak kulikjak changed the title Fix subprocess extra_groups gid conversion bpo-42655: Fix subprocess extra_groups gid conversion Dec 16, 2020
@gpshead
Copy link
Member

gpshead commented Dec 23, 2020

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?

@gpshead gpshead self-assigned this Dec 23, 2020
@kulikjak
Copy link
Contributor Author

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.

Yes, and uid_t & gid_t are still used one several other places, which confirms that it should be available on every supported platform.

Can you add a news entry using blurb or the blurb_it service?

Done

@serhiy-storchaka
Copy link
Member

i'm not sure why void* was ever used on those APIs

Because it is the signature of the converter function for "O&" format unit in PyArg_Parse. And many other converters have void* type of the se3cond parameter. But I agree that specifying more concrete pointer type is more convenient and absolute compatible.

@serhiy-storchaka serhiy-storchaka merged commit 0159e5e into python:master Dec 29, 2020
@serhiy-storchaka serhiy-storchaka added needs backport to 3.9 type-bug An unexpected behavior, bug, or error labels Dec 29, 2020
@miss-islington
Copy link
Contributor

Thanks @kulikjak for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 29, 2020
@bedevere-bot
Copy link

GH-23997 is a backport of this pull request to the 3.9 branch.

miss-islington added a commit that referenced this pull request Dec 29, 2020
(cherry picked from commit 0159e5e)

Co-authored-by: Jakub Kulík <[email protected]>
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants