From d1a1fbd09da55a1fefb55cc18339e76e384404de Mon Sep 17 00:00:00 2001 From: Jeffrey Kintscher Date: Thu, 2 May 2019 15:47:21 -0700 Subject: [PATCH 1/3] BPO-35070 test_posix fails on macOS 10.14 Mojave The test_getgroups subtest fails when the current user running the test belongs to the maximum number supplemental groups. It fails because posix.getgrouplist() passes an array to the POSIX API function getgrouplist() that is too small. It uses NGROUPS_MAX to determine the size of the array that it passes to getgrouplist() to be filled with the user's GIDs. NGROUPS_MAX is defined by POSIX.1 as the maximum number of supplimental groups a user can belong to. The value has to be increased by one because getgrouplist() returns both the supplemental groups and the primary group, i.e. all of the groups the user belongs to. NGROUPS_MAX is defined as 16 on OS X systems, so the test fails when the user belongs to 16 supplemental groups because it passes an array with 16 elements to receive the GIDs, but getgrouplist() returns an error because it wants to return 17 GIDs (the primary plus 16 supplemntals). This problem most likely is not seen on Linux systems because they typically have a much higher limit for supplemental groups (e.g. 65,536 on Ubuntu 16.04.4). --- Modules/posixmodule.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 56ec3ee5a0eed3..18070832dc44a0 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -6640,7 +6640,15 @@ static PyObject * posix_getgrouplist(PyObject *self, PyObject *args) { #ifdef NGROUPS_MAX -#define MAX_GROUPS NGROUPS_MAX + /* + * NGROUPS_MAX is defined by POSIX.1 as the maximum + * number of supplimental groups a users can belong to. + * We have to increment it by one because + * getgrouplist() returns both the supplemental groups + * and the primary group, i.e. all of the groups the + * user belongs to. + */ +#define MAX_GROUPS (NGROUPS_MAX + 1) #else /* defined to be 16 on Solaris7, so this should be a small number */ #define MAX_GROUPS 64 From 811cb59ae8ba7c71ad20e6b8a28a6462696940e9 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" Date: Thu, 9 May 2019 18:50:56 +0000 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../NEWS.d/next/Library/2019-05-09-18-50-55.bpo-35070.4vaqNL.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2019-05-09-18-50-55.bpo-35070.4vaqNL.rst diff --git a/Misc/NEWS.d/next/Library/2019-05-09-18-50-55.bpo-35070.4vaqNL.rst b/Misc/NEWS.d/next/Library/2019-05-09-18-50-55.bpo-35070.4vaqNL.rst new file mode 100644 index 00000000000000..0b17820e5a24d4 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-05-09-18-50-55.bpo-35070.4vaqNL.rst @@ -0,0 +1 @@ +posix.getgrouplist() fails when when the user has NGROUPS_MAX supplemental GIDs. This becomes a problem on OS X Mojave (14.x) because the system defines NGROUPS_MAX to be 16 and it is common for a user to belong to 16 groups. The fix is to increase the size of the array passed to the underlying getgrouplist() POSIX API function to allow space for the user's primary group, which is not accounted for in the NGROUPS_MAX constant (per POSIX.1). \ No newline at end of file From 39aeb0d6e3ab7642cd8161f3d7911fbee22bbca2 Mon Sep 17 00:00:00 2001 From: Jeffrey Kintscher Date: Mon, 3 Jun 2019 17:23:44 -0700 Subject: [PATCH 3/3] bpo-35070: define MAX_GROUPS in one place so that its meaning is consistent throughout the source file; update news blurb with requested changes --- .../2019-05-09-18-50-55.bpo-35070.4vaqNL.rst | 3 +- Modules/posixmodule.c | 40 ++++++++----------- 2 files changed, 19 insertions(+), 24 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2019-05-09-18-50-55.bpo-35070.4vaqNL.rst b/Misc/NEWS.d/next/Library/2019-05-09-18-50-55.bpo-35070.4vaqNL.rst index 0b17820e5a24d4..e823f1d469cca6 100644 --- a/Misc/NEWS.d/next/Library/2019-05-09-18-50-55.bpo-35070.4vaqNL.rst +++ b/Misc/NEWS.d/next/Library/2019-05-09-18-50-55.bpo-35070.4vaqNL.rst @@ -1 +1,2 @@ -posix.getgrouplist() fails when when the user has NGROUPS_MAX supplemental GIDs. This becomes a problem on OS X Mojave (14.x) because the system defines NGROUPS_MAX to be 16 and it is common for a user to belong to 16 groups. The fix is to increase the size of the array passed to the underlying getgrouplist() POSIX API function to allow space for the user's primary group, which is not accounted for in the NGROUPS_MAX constant (per POSIX.1). \ No newline at end of file +posix.getgrouplist() now works correctly when the user belongs to +NGROUPS_MAX supplemental groups. Patch by Jeffrey Kintscher. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 18070832dc44a0..cc3c5b99be4def 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -6627,6 +6627,13 @@ os_getpid_impl(PyObject *module) } #endif /* HAVE_GETPID */ +#ifdef NGROUPS_MAX +#define MAX_GROUPS NGROUPS_MAX +#else + /* defined to be 16 on Solaris7, so this should be a small number */ +#define MAX_GROUPS 64 +#endif + #ifdef HAVE_GETGROUPLIST /* AC 3.5: funny apple logic below */ @@ -6639,21 +6646,6 @@ Returns a list of groups to which a user belongs.\n\n\ static PyObject * posix_getgrouplist(PyObject *self, PyObject *args) { -#ifdef NGROUPS_MAX - /* - * NGROUPS_MAX is defined by POSIX.1 as the maximum - * number of supplimental groups a users can belong to. - * We have to increment it by one because - * getgrouplist() returns both the supplemental groups - * and the primary group, i.e. all of the groups the - * user belongs to. - */ -#define MAX_GROUPS (NGROUPS_MAX + 1) -#else - /* defined to be 16 on Solaris7, so this should be a small number */ -#define MAX_GROUPS 64 -#endif - const char *user; int i, ngroups; PyObject *list; @@ -6662,7 +6654,16 @@ posix_getgrouplist(PyObject *self, PyObject *args) #else gid_t *groups, basegid; #endif - ngroups = MAX_GROUPS; + + /* + * NGROUPS_MAX is defined by POSIX.1 as the maximum + * number of supplimental groups a users can belong to. + * We have to increment it by one because + * getgrouplist() returns both the supplemental groups + * and the primary group, i.e. all of the groups the + * user belongs to. + */ + ngroups = 1 + MAX_GROUPS; #ifdef __APPLE__ if (!PyArg_ParseTuple(args, "si:getgrouplist", &user, &basegid)) @@ -6731,13 +6732,6 @@ os_getgroups_impl(PyObject *module) /*[clinic end generated code: output=42b0c17758561b56 input=d3f109412e6a155c]*/ { PyObject *result = NULL; - -#ifdef NGROUPS_MAX -#define MAX_GROUPS NGROUPS_MAX -#else - /* defined to be 16 on Solaris7, so this should be a small number */ -#define MAX_GROUPS 64 -#endif gid_t grouplist[MAX_GROUPS]; /* On MacOSX getgroups(2) can return more than MAX_GROUPS results