Date: Tue, 10 Feb 2026 17:56:11 +0000 From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Cc: Dag-Erling=?utf-8?Q? Sm=C3=B8rg?=rav <des@FreeBSD.org> Subject: git: e4781e4e6d88 - releng/15.0 - blocklistd: Fix multiple bugs Message-ID: <698b713b.264d7.6229df2d@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch releng/15.0 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=e4781e4e6d88f73d6fe266eb520c240a343fafcf commit e4781e4e6d88f73d6fe266eb520c240a343fafcf Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2026-02-07 14:38:34 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2026-02-09 17:54:29 +0000 blocklistd: Fix multiple bugs * Fix file descriptor leak in the server * Fix race between parent and child in popenve() * Don't assume fdopen() can't fail Approved by: so Security: FreeBSD-SA-26:03.blocklistd Security: CVE-2026-2261 --- contrib/blocklist/bin/blacklistd.c | 8 +++-- contrib/blocklist/bin/blocklistd.c | 8 +++-- contrib/blocklist/port/popenve.c | 61 ++++++++++++++++++++------------------ 3 files changed, 42 insertions(+), 35 deletions(-) diff --git a/contrib/blocklist/bin/blacklistd.c b/contrib/blocklist/bin/blacklistd.c index cb6ce6578d9c..6021e70f214c 100644 --- a/contrib/blocklist/bin/blacklistd.c +++ b/contrib/blocklist/bin/blacklistd.c @@ -191,7 +191,7 @@ process(bl_t bl) } if (getremoteaddress(bi, &rss, &rsl) == -1) - return; + goto out; if (debug || bi->bi_msg[0]) { sockaddr_snprintf(rbuf, sizeof(rbuf), "%a:%p", (void *)&rss); @@ -204,12 +204,12 @@ process(bl_t bl) if (conf_find(bi->bi_fd, bi->bi_uid, &rss, &c) == NULL) { (*lfun)(LOG_DEBUG, "no rule matched"); - return; + goto out; } if (state_get(state, &c, &dbi) == -1) - return; + goto out; if (debug) { char b1[128], b2[128]; @@ -269,6 +269,8 @@ process(bl_t bl) state_put(state, &c, &dbi); out: + close(bi->bi_fd); + if (debug) { char b1[128], b2[128]; (*lfun)(LOG_DEBUG, "%s: final db state for %s: count=%d/%d " diff --git a/contrib/blocklist/bin/blocklistd.c b/contrib/blocklist/bin/blocklistd.c index 47c145c7aae1..ffa2ff2d74a6 100644 --- a/contrib/blocklist/bin/blocklistd.c +++ b/contrib/blocklist/bin/blocklistd.c @@ -191,7 +191,7 @@ process(bl_t bl) } if (getremoteaddress(bi, &rss, &rsl) == -1) - return; + goto out; if (debug || bi->bi_msg[0]) { sockaddr_snprintf(rbuf, sizeof(rbuf), "%a:%p", (void *)&rss); @@ -204,12 +204,12 @@ process(bl_t bl) if (conf_find(bi->bi_fd, bi->bi_uid, &rss, &c) == NULL) { (*lfun)(LOG_DEBUG, "no rule matched"); - return; + goto out; } if (state_get(state, &c, &dbi) == -1) - return; + goto out; if (debug) { char b1[128], b2[128]; @@ -269,6 +269,8 @@ process(bl_t bl) state_put(state, &c, &dbi); out: + close(bi->bi_fd); + if (debug) { char b1[128], b2[128]; (*lfun)(LOG_DEBUG, "%s: final db state for %s: count=%d/%d " diff --git a/contrib/blocklist/port/popenve.c b/contrib/blocklist/port/popenve.c index bdff8cdc1de4..e80058a8599a 100644 --- a/contrib/blocklist/port/popenve.c +++ b/contrib/blocklist/port/popenve.c @@ -111,11 +111,25 @@ pdes_get(int *pdes, const char **type) #endif } - if ((cur = malloc(sizeof(*cur))) != NULL) - return cur; + if ((cur = malloc(sizeof(*cur))) != NULL) { + if (**type == 'r') { + cur->fp = fdopen(pdes[0], *type); +#ifdef _REENTRANT + cur->fd = pdes[0]; +#endif + } else { + cur->fp = fdopen(pdes[1], *type); +#ifdef _REENTRANT + cur->fd = pdes[1]; +#endif + } + if (cur->fp != NULL) + return cur; + } serrno = errno; (void)close(pdes[0]); (void)close(pdes[1]); + free(cur); errno = serrno; return NULL; } @@ -125,16 +139,6 @@ pdes_child(int *pdes, const char *type) { struct pid *old; - /* POSIX.2 B.3.2.2 "popen() shall ensure that any streams - from previous popen() calls that remain open in the - parent process are closed in the new child process. */ - for (old = pidlist; old; old = old->next) -#ifdef _REENTRANT - (void)close(old->fd); /* don't allow a flush */ -#else - (void)close(fileno(old->fp)); /* don't allow a flush */ -#endif - if (type[0] == 'r') { (void)close(pdes[0]); if (pdes[1] != STDOUT_FILENO) { @@ -150,31 +154,30 @@ pdes_child(int *pdes, const char *type) (void)close(pdes[0]); } } + + /* POSIX.2 B.3.2.2 "popen() shall ensure that any streams + from previous popen() calls that remain open in the + parent process are closed in the new child process. */ + for (old = pidlist; old; old = old->next) { +#ifdef _REENTRANT + (void)close(old->fd); /* don't allow a flush */ +#else + (void)close(fileno(old->fp)); /* don't allow a flush */ +#endif + } } static void pdes_parent(int *pdes, struct pid *cur, pid_t pid, const char *type) { - FILE *iop; - - /* Parent; assume fdopen can't fail. */ - if (*type == 'r') { - iop = fdopen(pdes[0], type); -#ifdef _REENTRANT - cur->fd = pdes[0]; -#endif + /* Parent */ + if (*type == 'r') (void)close(pdes[1]); - } else { - iop = fdopen(pdes[1], type); -#ifdef _REENTRANT - cur->fd = pdes[1]; -#endif + else (void)close(pdes[0]); - } /* Link into list of file descriptors. */ - cur->fp = iop; - cur->pid = pid; + cur->pid = pid; cur->next = pidlist; pidlist = cur; } @@ -200,7 +203,7 @@ popenve(const char *cmd, char *const *argv, char *const *envp, const char *type) #ifdef _REENTRANT (void)rwlock_rdlock(&pidlist_lock); #endif - switch (pid = vfork()) { + switch (pid = fork()) { case -1: /* Error. */ serrno = errno; #ifdef _REENTRANThome | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?698b713b.264d7.6229df2d>