From ad24d6ca66aeb1928ac18bbc787402478db0799f Mon Sep 17 00:00:00 2001 From: Giampaolo Rodola Date: Wed, 20 Dec 2023 12:20:33 +0100 Subject: [PATCH] Fix NetBSD connections memory leak / core dump (#2341) --- HISTORY.rst | 4 ++ psutil/arch/netbsd/socks.c | 131 ++++++++++++++++++++++------------ psutil/tests/test_memleaks.py | 3 - 3 files changed, 88 insertions(+), 50 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 46591705b..c6ad7e144 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -3,6 +3,10 @@ 5.9.8 (IN DEVELOPMENT) ====================== +**Bug fixes** + +- 930_, [NetBSD], [critical]: `net_connections()`_ implementation was broken. + It could either leak memory or core dump. - 2340_, [NetBSD]: if process is terminated, `Process.cwd()`_ will return an empty string instead of raising `NoSuchProcess`_. diff --git a/psutil/arch/netbsd/socks.c b/psutil/arch/netbsd/socks.c index 08b0b7e67..5c8ad3bd6 100644 --- a/psutil/arch/netbsd/socks.c +++ b/psutil/arch/netbsd/socks.c @@ -44,6 +44,8 @@ enum af_filter { struct kif { SLIST_ENTRY(kif) kifs; struct kinfo_file *kif; + char *buf; + int has_buf; }; // kinfo_file results list @@ -54,36 +56,31 @@ SLIST_HEAD(kifhead, kif) kihead = SLIST_HEAD_INITIALIZER(kihead); struct kpcb { SLIST_ENTRY(kpcb) kpcbs; struct kinfo_pcb *kpcb; + struct kinfo_pcb *buf; + int has_buf; }; // kinfo_pcb results list SLIST_HEAD(kpcbhead, kpcb) kpcbhead = SLIST_HEAD_INITIALIZER(kpcbhead); -static void psutil_kiflist_init(void); -static void psutil_kiflist_clear(void); -static void psutil_kpcblist_init(void); -static void psutil_kpcblist_clear(void); -static int psutil_get_files(void); -static int psutil_get_sockets(const char *name); -static int psutil_get_info(int aff); - // Initialize kinfo_file results list. static void psutil_kiflist_init(void) { SLIST_INIT(&kihead); - return; } // Clear kinfo_file results list. static void psutil_kiflist_clear(void) { - while (!SLIST_EMPTY(&kihead)) { - SLIST_REMOVE_HEAD(&kihead, kifs); - } - - return; + while (!SLIST_EMPTY(&kihead)) { + struct kif *kif = SLIST_FIRST(&kihead); + if (kif->has_buf == 1) + free(kif->buf); + free(kif); + SLIST_REMOVE_HEAD(&kihead, kifs); + } } @@ -91,18 +88,19 @@ psutil_kiflist_clear(void) { static void psutil_kpcblist_init(void) { SLIST_INIT(&kpcbhead); - return; } // Clear kinof_pcb result list. static void psutil_kpcblist_clear(void) { - while (!SLIST_EMPTY(&kpcbhead)) { - SLIST_REMOVE_HEAD(&kpcbhead, kpcbs); - } - - return; + while (!SLIST_EMPTY(&kpcbhead)) { + struct kpcb *kpcb = SLIST_FIRST(&kpcbhead); + if (kpcb->has_buf == 1) + free(kpcb->buf); + free(kpcb); + SLIST_REMOVE_HEAD(&kpcbhead, kpcbs); + } } @@ -112,7 +110,7 @@ psutil_get_files(void) { size_t len; size_t j; int mib[6]; - char *buf; + char *buf = NULL; off_t offset; mib[0] = CTL_KERN; @@ -124,7 +122,7 @@ psutil_get_files(void) { if (sysctl(mib, 6, NULL, &len, NULL, 0) == -1) { PyErr_SetFromErrno(PyExc_OSError); - return -1; + goto error; } offset = len % sizeof(off_t); @@ -132,33 +130,46 @@ psutil_get_files(void) { if ((buf = malloc(len + offset)) == NULL) { PyErr_NoMemory(); - return -1; + goto error; } if (sysctl(mib, 6, buf + offset, &len, NULL, 0) == -1) { - free(buf); PyErr_SetFromErrno(PyExc_OSError); - return -1; + goto error; } len /= sizeof(struct kinfo_file); struct kinfo_file *ki = (struct kinfo_file *)(buf + offset); - for (j = 0; j < len; j++) { - struct kif *kif = malloc(sizeof(struct kif)); - kif->kif = &ki[j]; - SLIST_INSERT_HEAD(&kihead, kif, kifs); + if (len > 0) { + for (j = 0; j < len; j++) { + struct kif *kif = malloc(sizeof(struct kif)); + if (kif == NULL) { + PyErr_NoMemory(); + goto error; + } + kif->kif = &ki[j]; + if (j == 0) { + kif->has_buf = 1; + kif->buf = buf; + } + else { + kif->has_buf = 0; + kif->buf = NULL; + } + SLIST_INSERT_HEAD(&kihead, kif, kifs); + } } - - /* - // debug - struct kif *k; - SLIST_FOREACH(k, &kihead, kifs) { - printf("%d\n", k->kif->ki_pid); // NOQA + else { + free(buf); } - */ return 0; + +error: + if (buf != NULL) + free(buf); + return -1; } @@ -167,7 +178,7 @@ static int psutil_get_sockets(const char *name) { size_t namelen; int mib[8]; - struct kinfo_pcb *pcb; + struct kinfo_pcb *pcb = NULL; size_t len; size_t j; @@ -175,17 +186,17 @@ psutil_get_sockets(const char *name) { if (sysctlnametomib(name, mib, &namelen) == -1) { PyErr_SetFromErrno(PyExc_OSError); - return -1; + goto error; } if (sysctl(mib, __arraycount(mib), NULL, &len, NULL, 0) == -1) { PyErr_SetFromErrno(PyExc_OSError); - return -1; + goto error; } if ((pcb = malloc(len)) == NULL) { PyErr_NoMemory(); - return -1; + goto error; } memset(pcb, 0, len); @@ -193,20 +204,42 @@ psutil_get_sockets(const char *name) { mib[7] = len / sizeof(*pcb); if (sysctl(mib, __arraycount(mib), pcb, &len, NULL, 0) == -1) { - free(pcb); PyErr_SetFromErrno(PyExc_OSError); - return -1; + goto error; } len /= sizeof(struct kinfo_pcb); struct kinfo_pcb *kp = (struct kinfo_pcb *)pcb; - for (j = 0; j < len; j++) { - struct kpcb *kpcb = malloc(sizeof(struct kpcb)); - kpcb->kpcb = &kp[j]; - SLIST_INSERT_HEAD(&kpcbhead, kpcb, kpcbs); + if (len > 0) { + for (j = 0; j < len; j++) { + struct kpcb *kpcb = malloc(sizeof(struct kpcb)); + if (kpcb == NULL) { + PyErr_NoMemory(); + goto error; + } + kpcb->kpcb = &kp[j]; + if (j == 0) { + kpcb->has_buf = 1; + kpcb->buf = pcb; + } + else { + kpcb->has_buf = 0; + kpcb->buf = NULL; + } + SLIST_INSERT_HEAD(&kpcbhead, kpcb, kpcbs); + } + } + else { + free(pcb); } + return 0; + +error: + if (pcb != NULL) + free(pcb); + return -1; } @@ -318,6 +351,7 @@ psutil_net_connections(PyObject *self, PyObject *args) { psutil_kiflist_init(); psutil_kpcblist_init(); + if (psutil_get_files() != 0) goto error; if (psutil_get_info(ALL) != 0) @@ -429,8 +463,11 @@ psutil_net_connections(PyObject *self, PyObject *args) { return py_retlist; error: + psutil_kiflist_clear(); + psutil_kpcblist_clear(); + Py_DECREF(py_retlist); Py_XDECREF(py_tuple); Py_XDECREF(py_laddr); Py_XDECREF(py_raddr); - return 0; + return NULL; } diff --git a/psutil/tests/test_memleaks.py b/psutil/tests/test_memleaks.py index da3c37932..cd1b3f290 100755 --- a/psutil/tests/test_memleaks.py +++ b/psutil/tests/test_memleaks.py @@ -25,7 +25,6 @@ import psutil._common from psutil import LINUX from psutil import MACOS -from psutil import NETBSD from psutil import OPENBSD from psutil import POSIX from psutil import SUNOS @@ -249,7 +248,6 @@ def test_rlimit_set(self): # Windows implementation is based on a single system-wide # function (tested later). @unittest.skipIf(WINDOWS, "worthless on WINDOWS") - @unittest.skipIf(NETBSD, "critically broken on NETBSD (#930)") def test_connections(self): # TODO: UNIX sockets are temporarily implemented by parsing # 'pfiles' cmd output; we don't want that part of the code to @@ -422,7 +420,6 @@ def test_net_io_counters(self): @fewtimes_if_linux() @unittest.skipIf(MACOS and os.getuid() != 0, "need root access") - @unittest.skipIf(NETBSD, "critically broken on NETBSD (#930)") def test_net_connections(self): # always opens and handle on Windows() (once) psutil.net_connections(kind='all')