diff --git a/Misc/NEWS.d/next/Library/2024-11-20-11-37-08.gh-issue-126316.ElkZmE.rst b/Misc/NEWS.d/next/Library/2024-11-20-11-37-08.gh-issue-126316.ElkZmE.rst new file mode 100644 index 00000000000000..d643254c5b3564 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-11-20-11-37-08.gh-issue-126316.ElkZmE.rst @@ -0,0 +1,2 @@ +:mod:`grp`: Make :func:`grp.getgrall` thread-safe by adding a mutex. Patch +by Victor Stinner. diff --git a/Modules/clinic/grpmodule.c.h b/Modules/clinic/grpmodule.c.h index cc0ad210f42743..43700a11c15c91 100644 --- a/Modules/clinic/grpmodule.c.h +++ b/Modules/clinic/grpmodule.c.h @@ -2,35 +2,11 @@ preserve [clinic start generated code]*/ -PyDoc_STRVAR(grp_getgrgid__doc__, -"getgrgid($module, /, id)\n" -"--\n" -"\n" -"Return the group database entry for the given numeric group ID.\n" -"\n" -"If id is not valid, raise KeyError."); - -#define GRP_GETGRGID_METHODDEF \ - {"getgrgid", (PyCFunction)(void(*)(void))grp_getgrgid, METH_VARARGS|METH_KEYWORDS, grp_getgrgid__doc__}, - -static PyObject * -grp_getgrgid_impl(PyObject *module, PyObject *id); - -static PyObject * -grp_getgrgid(PyObject *module, PyObject *args, PyObject *kwargs) -{ - PyObject *return_value = NULL; - static char *_keywords[] = {"id", NULL}; - PyObject *id; - - if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O:getgrgid", _keywords, - &id)) - goto exit; - return_value = grp_getgrgid_impl(module, id); - -exit: - return return_value; -} +#if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) +# include "pycore_gc.h" // PyGC_Head +# include "pycore_runtime.h" // _Py_ID() +#endif +#include "pycore_modsupport.h" // _PyArg_UnpackKeywords() PyDoc_STRVAR(grp_getgrnam__doc__, "getgrnam($module, /, name)\n" @@ -41,21 +17,52 @@ PyDoc_STRVAR(grp_getgrnam__doc__, "If name is not valid, raise KeyError."); #define GRP_GETGRNAM_METHODDEF \ - {"getgrnam", (PyCFunction)(void(*)(void))grp_getgrnam, METH_VARARGS|METH_KEYWORDS, grp_getgrnam__doc__}, + {"getgrnam", _PyCFunction_CAST(grp_getgrnam), METH_FASTCALL|METH_KEYWORDS, grp_getgrnam__doc__}, static PyObject * grp_getgrnam_impl(PyObject *module, PyObject *name); static PyObject * -grp_getgrnam(PyObject *module, PyObject *args, PyObject *kwargs) +grp_getgrnam(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { PyObject *return_value = NULL; - static char *_keywords[] = {"name", NULL}; + #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) + + #define NUM_KEYWORDS 1 + static struct { + PyGC_Head _this_is_not_used; + PyObject_VAR_HEAD + PyObject *ob_item[NUM_KEYWORDS]; + } _kwtuple = { + .ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS) + .ob_item = { &_Py_ID(name), }, + }; + #undef NUM_KEYWORDS + #define KWTUPLE (&_kwtuple.ob_base.ob_base) + + #else // !Py_BUILD_CORE + # define KWTUPLE NULL + #endif // !Py_BUILD_CORE + + static const char * const _keywords[] = {"name", NULL}; + static _PyArg_Parser _parser = { + .keywords = _keywords, + .fname = "getgrnam", + .kwtuple = KWTUPLE, + }; + #undef KWTUPLE + PyObject *argsbuf[1]; PyObject *name; - if (!PyArg_ParseTupleAndKeywords(args, kwargs, "U:getgrnam", _keywords, - &name)) + args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 1, 1, 0, argsbuf); + if (!args) { + goto exit; + } + if (!PyUnicode_Check(args[0])) { + _PyArg_BadArgument("getgrnam", "argument 'name'", "str", args[0]); goto exit; + } + name = args[0]; return_value = grp_getgrnam_impl(module, name); exit: @@ -82,4 +89,4 @@ grp_getgrall(PyObject *module, PyObject *Py_UNUSED(ignored)) { return grp_getgrall_impl(module); } -/*[clinic end generated code: output=81f180beb67fc585 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=1168333f1b15de11 input=a9049054013a1b77]*/ diff --git a/Modules/grpmodule.c b/Modules/grpmodule.c index f7d3e12f347ec2..e0534576ff1e82 100644 --- a/Modules/grpmodule.c +++ b/Modules/grpmodule.c @@ -1,9 +1,8 @@ /* UNIX group file access module */ -// Need limited C API version 3.13 for PyMem_RawRealloc() -#include "pyconfig.h" // Py_GIL_DISABLED -#ifndef Py_GIL_DISABLED -#define Py_LIMITED_API 0x030d0000 +// Argument Clinic uses the internal C API +#ifndef Py_BUILD_CORE_BUILTIN +# define Py_BUILD_CORE_MODULE 1 #endif #include "Python.h" @@ -110,20 +109,15 @@ mkgrent(PyObject *module, struct group *p) return v; } -/*[clinic input] -grp.getgrgid - - id: object - -Return the group database entry for the given numeric group ID. - -If id is not valid, raise KeyError. -[clinic start generated code]*/ - static PyObject * -grp_getgrgid_impl(PyObject *module, PyObject *id) -/*[clinic end generated code: output=30797c289504a1ba input=15fa0e2ccf5cda25]*/ +grp_getgrgid(PyObject *module, PyObject *args, PyObject *kwargs) { + static char *kwlist[] = {"id", NULL}; + PyObject *id; + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O", kwlist, &id)) { + return NULL; + } + PyObject *retval = NULL; int nomem = 0; char *buf = NULL, *buf2 = NULL; @@ -190,6 +184,15 @@ grp_getgrgid_impl(PyObject *module, PyObject *id) return retval; } +PyDoc_STRVAR(grp_getgrgid__doc__, +"getgrgid($module, /, id)\n" +"--\n" +"\n" +"Return the group database entry for the given numeric group ID.\n" +"\n" +"If id is not valid, raise KeyError."); + + /*[clinic input] grp.getgrnam @@ -281,28 +284,38 @@ static PyObject * grp_getgrall_impl(PyObject *module) /*[clinic end generated code: output=585dad35e2e763d7 input=d7df76c825c367df]*/ { - PyObject *d; - struct group *p; - - if ((d = PyList_New(0)) == NULL) + PyObject *d = PyList_New(0); + if (d == NULL) { return NULL; + } + + static PyMutex getgrall_mutex = {0}; + PyMutex_Lock(&getgrall_mutex); setgrent(); + + struct group *p; while ((p = getgrent()) != NULL) { + // gh-126316: Don't release the mutex around mkgrent() since + // setgrent()/endgrent() are not reentrant / thread-safe. A deadlock + // is unlikely since mkgrent() should not be able to call arbitrary + // Python code. PyObject *v = mkgrent(module, p); if (v == NULL || PyList_Append(d, v) != 0) { Py_XDECREF(v); - Py_DECREF(d); - endgrent(); - return NULL; + Py_CLEAR(d); + goto done; } Py_DECREF(v); } + +done: endgrent(); + PyMutex_Unlock(&getgrall_mutex); return d; } static PyMethodDef grp_methods[] = { - GRP_GETGRGID_METHODDEF + {"getgrgid", _PyCFunction_CAST(grp_getgrgid), METH_VARARGS|METH_KEYWORDS, grp_getgrgid__doc__}, GRP_GETGRNAM_METHODDEF GRP_GETGRALL_METHODDEF {NULL, NULL} diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index 260eaa4f14f767..5e271a5014cb35 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -742,6 +742,7 @@ Modules/expat/xmlrole.c - declClose - Modules/expat/xmlrole.c - error - ## other +Modules/grpmodule.c grp_getgrall_impl getgrall_mutex - Modules/_io/_iomodule.c - _PyIO_Module - Modules/_sqlite/module.c - _sqlite3module - Modules/clinic/md5module.c.h _md5_md5 _keywords -