Fix subprocess extra_groups gid conversion

[kulikjak]

BPO	42655
Nosy	@gpshead, @serhiy-storchaka, @izbyshev, @miss-islington, @kulikjak
PRs	bpo-42655: Fix subprocess extra_groups gid conversion #23762 [3.9] bpo-42655: Fix subprocess extra_groups gid conversion (GH-23762) #23997

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-12-29.13:41:01.746>
created_at = <Date 2020-12-16.10:05:22.776>
labels = ['extension-modules', '3.9', '3.10']
title = 'Fix subprocess extra_groups gid conversion'
updated_at = <Date 2020-12-29.13:41:01.745>
user = 'https://github.com/kulikjak'

bugs.python.org fields:

activity = <Date 2020-12-29.13:41:01.745>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2020-12-29.13:41:01.746>
closer = 'serhiy.storchaka'
components = ['Extension Modules']
creation = <Date 2020-12-16.10:05:22.776>
creator = 'kulikjak'
dependencies = []
files = []
hgrepos = []
issue_num = 42655
keywords = ['patch']
message_count = 6.0
messages = ['383132', '383380', '383431', '383995', '383999', '384004']
nosy_count = 5.0
nosy_names = ['gregory.p.smith', 'serhiy.storchaka', 'izbyshev', 'miss-islington', 'kulikjak']
pr_nums = ['23762', '23997']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue42655'
versions = ['Python 3.9', 'Python 3.10']

[kulikjak]

C function subprocess_fork_exec incorrectly transforms gids from the 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.

[izbyshev]

This bug would have been caught at compile time if _Py_Gid_Converter() used gid_t * instead of void *. I couldn't find any call sites where void * would be needed, so probably _Py_Gid_Converter() should be fixed too (in a separate PR/issue?). The same applies to _Py_Uid_Converter().

[kulikjak]

I checked and indeed there seems to be no reason as for why should we use void * rather than gid_t * and uid_t *. I changed that in the attached PR.

[serhiy-storchaka]

New changeset 0159e5e by Jakub Kulík in branch 'master':
bpo-42655: Fix subprocess extra_groups gid conversion (GH-23762)
0159e5e

[miss-islington]

New changeset 3966e2e by Miss Islington (bot) in branch '3.9':
bpo-42655: Fix subprocess extra_groups gid conversion (GH-23762)
3966e2e

[serhiy-storchaka]

Thank you for your contribution Jakub.