From fe28011b941fa104189d7f2d575a0bfe2f653ea3 Mon Sep 17 00:00:00 2001 From: tonghuaroot Date: Sun, 21 Jun 2026 15:06:28 +0800 Subject: [PATCH 1/2] gh-74667: Make socket.getservbyname(), getservbyport() and getprotobyname() thread-safe --- Lib/test/test_socket.py | 50 +++++++ ...6-06-21-15-30-00.gh-issue-74667.Impmte.rst | 2 + Modules/socketmodule.c | 136 ++++++++++++++++-- configure | 122 ++++++++++++++++ configure.ac | 56 ++++++++ pyconfig.h.in | 9 ++ 6 files changed, 365 insertions(+), 10 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-06-21-15-30-00.gh-issue-74667.Impmte.rst diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 63c465e3bfea16..f866ede2bdcc98 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -7528,6 +7528,56 @@ def detach(): with threading_helper.start_threads([t1, t2]): pass + def test_getservby_getprotobyname_race(self): + # gh-74667: these used to share a static buffer with no lock, so + # concurrent calls clobbered each other's result. + try: + http_port = socket.getservbyname('http', 'tcp') + https_port = socket.getservbyname('https', 'tcp') + http_name = socket.getservbyport(http_port, 'tcp') + https_name = socket.getservbyport(https_port, 'tcp') + tcp = socket.getprotobyname('tcp') + udp = socket.getprotobyname('udp') + except OSError: + self.skipTest('required services/protocols are not available') + + loops = 10000 + errors = [] + + def check_servbyname(name, proto, expected): + for _ in range(loops): + if socket.getservbyname(name, proto) != expected: + errors.append('getservbyname') + return + + def check_servbyport(port, proto, expected): + for _ in range(loops): + if socket.getservbyport(port, proto) != expected: + errors.append('getservbyport') + return + + def check_protobyname(name, expected): + for _ in range(loops): + if socket.getprotobyname(name) != expected: + errors.append('getprotobyname') + return + + threads = [ + threading.Thread(target=check_servbyname, + args=('http', 'tcp', http_port)), + threading.Thread(target=check_servbyname, + args=('https', 'tcp', https_port)), + threading.Thread(target=check_servbyport, + args=(http_port, 'tcp', http_name)), + threading.Thread(target=check_servbyport, + args=(https_port, 'tcp', https_name)), + threading.Thread(target=check_protobyname, args=('tcp', tcp)), + threading.Thread(target=check_protobyname, args=('udp', udp)), + ] + with threading_helper.start_threads(threads): + pass + self.assertEqual(errors, []) + class ReentrantMutationTests(unittest.TestCase): """Regression tests for re-entrant mutation in sendmsg/recvmsg_into. diff --git a/Misc/NEWS.d/next/Library/2026-06-21-15-30-00.gh-issue-74667.Impmte.rst b/Misc/NEWS.d/next/Library/2026-06-21-15-30-00.gh-issue-74667.Impmte.rst new file mode 100644 index 00000000000000..00a0fae62600a8 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-06-21-15-30-00.gh-issue-74667.Impmte.rst @@ -0,0 +1,2 @@ +Made :func:`socket.getservbyname`, :func:`socket.getservbyport` and +:func:`socket.getprotobyname` thread-safe. diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 3e82af3194d053..ab97ce7bc5cdfd 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -205,6 +205,19 @@ shutdown(how) -- shut down traffic in one or both directions\n\ # define USE_GETHOSTBYNAME_LOCK #endif +/* These return a pointer to a static buffer shared between threads; a lock is + needed wherever the reentrant *_r() variants are unavailable. */ +#if (!defined(HAVE_GETSERVBYNAME_R) || !defined(HAVE_GETSERVBYPORT_R) || \ + !defined(HAVE_GETPROTOBYNAME_R)) && !defined(MS_WINDOWS) +# define USE_GETSERVBYNAME_LOCK +#endif + +/* netdb_lock is needed if any of the netdb lookups falls back to the + non-reentrant function. */ +#if defined(USE_GETHOSTBYNAME_LOCK) || defined(USE_GETSERVBYNAME_LOCK) +# define USE_NETDB_LOCK +#endif + #if defined(__APPLE__) || defined(__CYGWIN__) || defined(__NetBSD__) # include #endif @@ -1168,8 +1181,9 @@ new_sockobject(socket_state *state, SOCKET_T fd, int family, int type, /* Lock to allow python interpreter to continue, but only allow one - thread to be in gethostbyname or getaddrinfo */ -#if defined(USE_GETHOSTBYNAME_LOCK) + thread to be in gethostbyname, getaddrinfo, getservby*() or + getprotobyname() */ +#if defined(USE_NETDB_LOCK) static PyThread_type_lock netdb_lock; #endif @@ -6361,6 +6375,13 @@ socket_getservbyname(PyObject *self, PyObject *args) { const char *name, *proto=NULL; struct servent *sp; + PyObject *ret = NULL; +#ifdef HAVE_GETSERVBYNAME_R + struct servent serv; + char *buf = NULL; + size_t buf_len = 1024; + int err = 0; +#endif if (!PyArg_ParseTuple(args, "s|s:getservbyname", &name, &proto)) return NULL; @@ -6368,14 +6389,41 @@ socket_getservbyname(PyObject *self, PyObject *args) return NULL; } +#ifdef HAVE_GETSERVBYNAME_R + do { + char *new_buf = PyMem_RawRealloc(buf, buf_len); + if (new_buf == NULL) { + PyMem_RawFree(buf); + return PyErr_NoMemory(); + } + buf = new_buf; + Py_BEGIN_ALLOW_THREADS + err = getservbyname_r(name, proto, &serv, buf, buf_len, &sp); + Py_END_ALLOW_THREADS + buf_len *= 2; + } while (err == ERANGE); +#else Py_BEGIN_ALLOW_THREADS +#ifdef USE_GETSERVBYNAME_LOCK + PyThread_acquire_lock(netdb_lock, 1); +#endif sp = getservbyname(name, proto); Py_END_ALLOW_THREADS +#endif /* HAVE_GETSERVBYNAME_R */ if (sp == NULL) { PyErr_SetString(PyExc_OSError, "service/proto not found"); - return NULL; } - return PyLong_FromLong((long) ntohs(sp->s_port)); + else { + ret = PyLong_FromLong((long) ntohs(sp->s_port)); + } +#ifdef HAVE_GETSERVBYNAME_R + PyMem_RawFree(buf); +#else +#ifdef USE_GETSERVBYNAME_LOCK + PyThread_release_lock(netdb_lock); +#endif +#endif /* HAVE_GETSERVBYNAME_R */ + return ret; } PyDoc_STRVAR(getservbyname_doc, @@ -6398,6 +6446,13 @@ socket_getservbyport(PyObject *self, PyObject *args) int port; const char *proto=NULL; struct servent *sp; + PyObject *ret = NULL; +#ifdef HAVE_GETSERVBYPORT_R + struct servent serv; + char *buf = NULL; + size_t buf_len = 1024; + int err = 0; +#endif if (!PyArg_ParseTuple(args, "i|s:getservbyport", &port, &proto)) return NULL; if (port < 0 || port > 0xffff) { @@ -6411,14 +6466,42 @@ socket_getservbyport(PyObject *self, PyObject *args) return NULL; } +#ifdef HAVE_GETSERVBYPORT_R + do { + char *new_buf = PyMem_RawRealloc(buf, buf_len); + if (new_buf == NULL) { + PyMem_RawFree(buf); + return PyErr_NoMemory(); + } + buf = new_buf; + Py_BEGIN_ALLOW_THREADS + err = getservbyport_r(htons((short)port), proto, &serv, buf, buf_len, + &sp); + Py_END_ALLOW_THREADS + buf_len *= 2; + } while (err == ERANGE); +#else Py_BEGIN_ALLOW_THREADS +#ifdef USE_GETSERVBYNAME_LOCK + PyThread_acquire_lock(netdb_lock, 1); +#endif sp = getservbyport(htons((short)port), proto); Py_END_ALLOW_THREADS +#endif /* HAVE_GETSERVBYPORT_R */ if (sp == NULL) { PyErr_SetString(PyExc_OSError, "port/proto not found"); - return NULL; } - return PyUnicode_FromString(sp->s_name); + else { + ret = PyUnicode_FromString(sp->s_name); + } +#ifdef HAVE_GETSERVBYPORT_R + PyMem_RawFree(buf); +#else +#ifdef USE_GETSERVBYNAME_LOCK + PyThread_release_lock(netdb_lock); +#endif +#endif /* HAVE_GETSERVBYPORT_R */ + return ret; } PyDoc_STRVAR(getservbyport_doc, @@ -6440,16 +6523,50 @@ socket_getprotobyname(PyObject *self, PyObject *args) { const char *name; struct protoent *sp; + PyObject *ret = NULL; +#ifdef HAVE_GETPROTOBYNAME_R + struct protoent proto; + char *buf = NULL; + size_t buf_len = 1024; + int err = 0; +#endif if (!PyArg_ParseTuple(args, "s:getprotobyname", &name)) return NULL; +#ifdef HAVE_GETPROTOBYNAME_R + do { + char *new_buf = PyMem_RawRealloc(buf, buf_len); + if (new_buf == NULL) { + PyMem_RawFree(buf); + return PyErr_NoMemory(); + } + buf = new_buf; + Py_BEGIN_ALLOW_THREADS + err = getprotobyname_r(name, &proto, buf, buf_len, &sp); + Py_END_ALLOW_THREADS + buf_len *= 2; + } while (err == ERANGE); +#else Py_BEGIN_ALLOW_THREADS +#ifdef USE_GETSERVBYNAME_LOCK + PyThread_acquire_lock(netdb_lock, 1); +#endif sp = getprotobyname(name); Py_END_ALLOW_THREADS +#endif /* HAVE_GETPROTOBYNAME_R */ if (sp == NULL) { PyErr_SetString(PyExc_OSError, "protocol not found"); - return NULL; } - return PyLong_FromLong((long) sp->p_proto); + else { + ret = PyLong_FromLong((long) sp->p_proto); + } +#ifdef HAVE_GETPROTOBYNAME_R + PyMem_RawFree(buf); +#else +#ifdef USE_GETSERVBYNAME_LOCK + PyThread_release_lock(netdb_lock); +#endif +#endif /* HAVE_GETPROTOBYNAME_R */ + return ret; } PyDoc_STRVAR(getprotobyname_doc, @@ -9292,8 +9409,7 @@ socket_exec(PyObject *m) #endif #endif /* _MSTCPIP_ */ - /* Initialize gethostbyname lock */ -#if defined(USE_GETHOSTBYNAME_LOCK) +#if defined(USE_NETDB_LOCK) netdb_lock = PyThread_allocate_lock(); if (netdb_lock == NULL) { goto error; diff --git a/configure b/configure index 12fd0d15698ac1..182b1724606df3 100755 --- a/configure +++ b/configure @@ -26584,6 +26584,128 @@ fi +# Check for the reentrant getservbyname_r(), getservbyport_r() and +# getprotobyname_r() functions. Unlike gethostbyname_r() they have a single, +# POSIX-style signature, so a compile test with that prototype is enough. +OLD_CFLAGS=$CFLAGS +CFLAGS="$CFLAGS $MY_CPPFLAGS $MY_THREAD_CPPFLAGS $MY_CFLAGS" + +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for getservbyname_r" >&5 +printf %s "checking for getservbyname_r... " >&6; } +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +#include + +int +main (void) +{ + + char buffer[2048]; + struct servent serv, *res; + + (void) getservbyname_r("name", "proto", &serv, buffer, sizeof(buffer), &res); + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO" +then : + + +printf "%s\n" "#define HAVE_GETSERVBYNAME_R 1" >>confdefs.h + + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: yes" >&5 +printf "%s\n" "yes" >&6; } + +else case e in #( + e) + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: no" >&5 +printf "%s\n" "no" >&6; } + ;; +esac +fi +rm -f core conftest.err conftest.$ac_objext conftest.beam conftest.$ac_ext + +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for getservbyport_r" >&5 +printf %s "checking for getservbyport_r... " >&6; } +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +#include + +int +main (void) +{ + + char buffer[2048]; + struct servent serv, *res; + + (void) getservbyport_r(80, "proto", &serv, buffer, sizeof(buffer), &res); + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO" +then : + + +printf "%s\n" "#define HAVE_GETSERVBYPORT_R 1" >>confdefs.h + + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: yes" >&5 +printf "%s\n" "yes" >&6; } + +else case e in #( + e) + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: no" >&5 +printf "%s\n" "no" >&6; } + ;; +esac +fi +rm -f core conftest.err conftest.$ac_objext conftest.beam conftest.$ac_ext + +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for getprotobyname_r" >&5 +printf %s "checking for getprotobyname_r... " >&6; } +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +#include + +int +main (void) +{ + + char buffer[2048]; + struct protoent prot, *res; + + (void) getprotobyname_r("name", &prot, buffer, sizeof(buffer), &res); + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO" +then : + + +printf "%s\n" "#define HAVE_GETPROTOBYNAME_R 1" >>confdefs.h + + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: yes" >&5 +printf "%s\n" "yes" >&6; } + +else case e in #( + e) + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: no" >&5 +printf "%s\n" "no" >&6; } + ;; +esac +fi +rm -f core conftest.err conftest.$ac_objext conftest.beam conftest.$ac_ext + +CFLAGS=$OLD_CFLAGS + # checks for system services # (none yet) diff --git a/configure.ac b/configure.ac index 3b42fdfe40385d..f5924d00459079 100644 --- a/configure.ac +++ b/configure.ac @@ -6210,6 +6210,62 @@ AC_SUBST([HAVE_GETHOSTBYNAME_R_3_ARG]) AC_SUBST([HAVE_GETHOSTBYNAME_R]) AC_SUBST([HAVE_GETHOSTBYNAME]) +# Check for the reentrant getservbyname_r(), getservbyport_r() and +# getprotobyname_r() functions. Unlike gethostbyname_r() they have a single, +# POSIX-style signature, so a compile test with that prototype is enough. +OLD_CFLAGS=$CFLAGS +CFLAGS="$CFLAGS $MY_CPPFLAGS $MY_THREAD_CPPFLAGS $MY_CFLAGS" + +AC_MSG_CHECKING([for getservbyname_r]) +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ +#include +]], [[ + char buffer[2048]; + struct servent serv, *res; + + (void) getservbyname_r("name", "proto", &serv, buffer, sizeof(buffer), &res); +]])], [ + AC_DEFINE([HAVE_GETSERVBYNAME_R], [1], + [Define this if you have the reentrant version of getservbyname_r().]) + AC_MSG_RESULT([yes]) +], [ + AC_MSG_RESULT([no]) +]) + +AC_MSG_CHECKING([for getservbyport_r]) +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ +#include +]], [[ + char buffer[2048]; + struct servent serv, *res; + + (void) getservbyport_r(80, "proto", &serv, buffer, sizeof(buffer), &res); +]])], [ + AC_DEFINE([HAVE_GETSERVBYPORT_R], [1], + [Define this if you have the reentrant version of getservbyport_r().]) + AC_MSG_RESULT([yes]) +], [ + AC_MSG_RESULT([no]) +]) + +AC_MSG_CHECKING([for getprotobyname_r]) +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ +#include +]], [[ + char buffer[2048]; + struct protoent prot, *res; + + (void) getprotobyname_r("name", &prot, buffer, sizeof(buffer), &res); +]])], [ + AC_DEFINE([HAVE_GETPROTOBYNAME_R], [1], + [Define this if you have the reentrant version of getprotobyname_r().]) + AC_MSG_RESULT([yes]) +], [ + AC_MSG_RESULT([no]) +]) + +CFLAGS=$OLD_CFLAGS + # checks for system services # (none yet) diff --git a/pyconfig.h.in b/pyconfig.h.in index 2bef8d38497c54..c4aa5af7179f0f 100644 --- a/pyconfig.h.in +++ b/pyconfig.h.in @@ -588,6 +588,9 @@ /* Define if you have the 'getprotobyname' function. */ #undef HAVE_GETPROTOBYNAME +/* Define this if you have the reentrant version of getprotobyname_r(). */ +#undef HAVE_GETPROTOBYNAME_R + /* Define to 1 if you have the 'getpwent' function. */ #undef HAVE_GETPWENT @@ -618,9 +621,15 @@ /* Define if you have the 'getservbyname' function. */ #undef HAVE_GETSERVBYNAME +/* Define this if you have the reentrant version of getservbyname_r(). */ +#undef HAVE_GETSERVBYNAME_R + /* Define if you have the 'getservbyport' function. */ #undef HAVE_GETSERVBYPORT +/* Define this if you have the reentrant version of getservbyport_r(). */ +#undef HAVE_GETSERVBYPORT_R + /* Define to 1 if you have the 'getsid' function. */ #undef HAVE_GETSID From e2b934d2566268385ff0f58822c530fb82b75243 Mon Sep 17 00:00:00 2001 From: tonghuaroot Date: Sun, 21 Jun 2026 22:33:25 +0800 Subject: [PATCH 2/2] gh-74667: Address review: larger netdb buffer, clearer probe comment and test diagnostics --- Lib/test/test_socket.py | 18 ++++++++++++------ Modules/socketmodule.c | 6 +++--- configure | 5 +++-- configure.ac | 5 +++-- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index f866ede2bdcc98..a4e9dc6103c916 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -7546,20 +7546,26 @@ def test_getservby_getprotobyname_race(self): def check_servbyname(name, proto, expected): for _ in range(loops): - if socket.getservbyname(name, proto) != expected: - errors.append('getservbyname') + got = socket.getservbyname(name, proto) + if got != expected: + errors.append(f'getservbyname({name!r}, {proto!r}): ' + f'{got!r} != {expected!r}') return def check_servbyport(port, proto, expected): for _ in range(loops): - if socket.getservbyport(port, proto) != expected: - errors.append('getservbyport') + got = socket.getservbyport(port, proto) + if got != expected: + errors.append(f'getservbyport({port!r}, {proto!r}): ' + f'{got!r} != {expected!r}') return def check_protobyname(name, expected): for _ in range(loops): - if socket.getprotobyname(name) != expected: - errors.append('getprotobyname') + got = socket.getprotobyname(name) + if got != expected: + errors.append(f'getprotobyname({name!r}): ' + f'{got!r} != {expected!r}') return threads = [ diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index ab97ce7bc5cdfd..8db48453dd7ca4 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -6379,7 +6379,7 @@ socket_getservbyname(PyObject *self, PyObject *args) #ifdef HAVE_GETSERVBYNAME_R struct servent serv; char *buf = NULL; - size_t buf_len = 1024; + size_t buf_len = 16384; int err = 0; #endif if (!PyArg_ParseTuple(args, "s|s:getservbyname", &name, &proto)) @@ -6450,7 +6450,7 @@ socket_getservbyport(PyObject *self, PyObject *args) #ifdef HAVE_GETSERVBYPORT_R struct servent serv; char *buf = NULL; - size_t buf_len = 1024; + size_t buf_len = 16384; int err = 0; #endif if (!PyArg_ParseTuple(args, "i|s:getservbyport", &port, &proto)) @@ -6527,7 +6527,7 @@ socket_getprotobyname(PyObject *self, PyObject *args) #ifdef HAVE_GETPROTOBYNAME_R struct protoent proto; char *buf = NULL; - size_t buf_len = 1024; + size_t buf_len = 16384; int err = 0; #endif if (!PyArg_ParseTuple(args, "s:getprotobyname", &name)) diff --git a/configure b/configure index 182b1724606df3..01aa6770d4c95b 100755 --- a/configure +++ b/configure @@ -26585,8 +26585,9 @@ fi # Check for the reentrant getservbyname_r(), getservbyport_r() and -# getprotobyname_r() functions. Unlike gethostbyname_r() they have a single, -# POSIX-style signature, so a compile test with that prototype is enough. +# getprotobyname_r() functions. These compile tests use the glibc/POSIX-TSF +# 6-argument form; platforms with a different arity (e.g. the 5-argument +# Solaris/illumos form) fail the probe and fall back to the netdb lock. OLD_CFLAGS=$CFLAGS CFLAGS="$CFLAGS $MY_CPPFLAGS $MY_THREAD_CPPFLAGS $MY_CFLAGS" diff --git a/configure.ac b/configure.ac index f5924d00459079..c1243fc5b5f150 100644 --- a/configure.ac +++ b/configure.ac @@ -6211,8 +6211,9 @@ AC_SUBST([HAVE_GETHOSTBYNAME_R]) AC_SUBST([HAVE_GETHOSTBYNAME]) # Check for the reentrant getservbyname_r(), getservbyport_r() and -# getprotobyname_r() functions. Unlike gethostbyname_r() they have a single, -# POSIX-style signature, so a compile test with that prototype is enough. +# getprotobyname_r() functions. These compile tests use the glibc/POSIX-TSF +# 6-argument form; platforms with a different arity (e.g. the 5-argument +# Solaris/illumos form) fail the probe and fall back to the netdb lock. OLD_CFLAGS=$CFLAGS CFLAGS="$CFLAGS $MY_CPPFLAGS $MY_THREAD_CPPFLAGS $MY_CFLAGS"