Date: Mon, 12 Jan 2026 03:42:30 +0000 From: Jake Freeland <jfree@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 60ae4e52f33e - main - syslogd: Terminate pipe processes gracefully Message-ID: <69646da6.f507.271aa284@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by jfree: URL: https://cgit.FreeBSD.org/src/commit/?id=60ae4e52f33e3c67720b68a29e35f6c114a3386c commit 60ae4e52f33e3c67720b68a29e35f6c114a3386c Author: Jake Freeland <jfree@FreeBSD.org> AuthorDate: 2025-12-22 06:05:37 +0000 Commit: Jake Freeland <jfree@FreeBSD.org> CommitDate: 2026-01-12 03:40:23 +0000 syslogd: Terminate pipe processes gracefully Pipe actions spawn a process based on the command provided in the syslogd configuration file. When a HUP signal is received, enter the process into the deadq instead of immediately killing it. This matches the behavior of syslogd prior to it being Capsicumized. Fixes: d2d180fb7736 --- usr.sbin/syslogd/syslogd.c | 94 +++++++++++++--------------------- usr.sbin/syslogd/tests/syslogd_test.sh | 34 ++++++++++++ 2 files changed, 70 insertions(+), 58 deletions(-) diff --git a/usr.sbin/syslogd/syslogd.c b/usr.sbin/syslogd/syslogd.c index e06464c0e749..f109fcd02563 100644 --- a/usr.sbin/syslogd/syslogd.c +++ b/usr.sbin/syslogd/syslogd.c @@ -370,43 +370,19 @@ static void increase_rcvbuf(int); static void close_filed(struct filed *f) { - switch (f->f_type) { - case F_FORW: - if (f->f_addr_fds != NULL) { - free(f->f_addrs); - for (size_t i = 0; i < f->f_num_addr_fds; ++i) - close(f->f_addr_fds[i]); - free(f->f_addr_fds); - f->f_addr_fds = NULL; - f->f_num_addr_fds = 0; - } - /* FALLTHROUGH */ - case F_FILE: - case F_TTY: - case F_CONSOLE: - f->f_type = F_UNUSED; - break; - case F_PIPE: - if (f->f_procdesc != -1) { - /* - * Close the procdesc, killing the underlying - * process (if it is still alive). - */ - (void)close(f->f_procdesc); - f->f_procdesc = -1; - /* - * The pipe process is guaranteed to be dead now, - * so remove it from the deadq. - */ - if (f->f_dq != NULL) { - deadq_remove(f->f_dq); - f->f_dq = NULL; - } - } - break; - default: - break; + if (f->f_type == F_FORW && f->f_addr_fds != NULL) { + free(f->f_addrs); + for (size_t i = 0; i < f->f_num_addr_fds; ++i) + close(f->f_addr_fds[i]); + free(f->f_addr_fds); + f->f_addr_fds = NULL; + f->f_num_addr_fds = 0; + } else if (f->f_type == F_PIPE && f->f_procdesc != -1) { + f->f_dq = deadq_enter(f->f_procdesc); } + + f->f_type = F_UNUSED; + if (f->f_file != -1) (void)close(f->f_file); f->f_file = -1; @@ -820,8 +796,23 @@ main(int argc, char *argv[]) break; case EVFILT_PROCDESC: if ((ev.fflags & NOTE_EXIT) != 0) { - log_deadchild(ev.ident, ev.data, ev.udata); - close_filed(ev.udata); + struct filed *f = ev.udata; + + log_deadchild(f->f_procdesc, ev.data, f); + (void)close(f->f_procdesc); + + f->f_procdesc = -1; + if (f->f_dq != NULL) { + deadq_remove(f->f_dq); + f->f_dq = NULL; + } + + /* + * If it is unused, then it was already closed. + * Free the file data in this case. + */ + if (f->f_type == F_UNUSED) + free(f); } break; } @@ -2272,9 +2263,6 @@ die(int signo) /* flush any pending output */ if (f->f_prevcount) fprintlog_successive(f, 0); - /* terminate existing pipe processes */ - if (f->f_type == F_PIPE) - close_filed(f); } if (signo) { dprintf("syslogd: exiting on signal %d\n", signo); @@ -2517,23 +2505,7 @@ closelogfiles(void) case F_FORW: case F_CONSOLE: case F_TTY: - close_filed(f); - break; case F_PIPE: - if (f->f_procdesc != -1) { - struct kevent ev; - /* - * This filed is going to be freed. - * Delete procdesc kevents that reference it. - */ - EV_SET(&ev, f->f_procdesc, EVFILT_PROCDESC, - EV_DELETE, NOTE_EXIT, 0, f); - if (kevent(kq, &ev, 1, NULL, 0, NULL) == -1) { - logerror("failed to delete procdesc" - "kevent"); - exit(1); - } - } close_filed(f); break; default: @@ -2554,7 +2526,13 @@ closelogfiles(void) } free(f->f_prop_filter); } - free(f); + + /* + * If a piped process is running, then defer the filed + * cleanup until it exits. + */ + if (f->f_type != F_PIPE || f->f_procdesc == -1) + free(f); } } diff --git a/usr.sbin/syslogd/tests/syslogd_test.sh b/usr.sbin/syslogd/tests/syslogd_test.sh index 2d093dd80c35..5422e78f6831 100644 --- a/usr.sbin/syslogd/tests/syslogd_test.sh +++ b/usr.sbin/syslogd/tests/syslogd_test.sh @@ -313,6 +313,39 @@ pipe_action_cleanup() syslogd_stop } +atf_test_case "pipe_action_reload" "cleanup" +pipe_action_reload_head() +{ + atf_set descr "Pipe processes terminate gracefully on reload" +} +pipe_action_reload_body() +{ + local logfile="${PWD}/pipe_reload.log" + local pipecmd="${PWD}/pipe_cmd.sh" + + cat <<__EOF__ > "${pipecmd}" +#!/bin/sh +echo START > ${logfile} +while read msg; do + echo \${msg} >> ${logfile} +done +echo END >> ${logfile} +exit 0 +__EOF__ + chmod +x "${pipecmd}" + + printf "!pipe\nuser.debug\t| %s\n" "${pipecmd}" > "${SYSLOGD_CONFIG}" + syslogd_start + + syslogd_log -p user.debug -t "pipe" -h "${SYSLOGD_LOCAL_SOCKET}" "MSG" + syslogd_reload + atf_check -s exit:0 -o match:"END" tail -n 1 "${logfile}" +} +pipe_action_reload_cleanup() +{ + syslogd_stop +} + atf_test_case "jail_noinet" "cleanup" jail_noinet_head() { @@ -561,6 +594,7 @@ atf_init_test_cases() atf_add_test_case "prop_filter" atf_add_test_case "host_action" atf_add_test_case "pipe_action" + atf_add_test_case "pipe_action_reload" atf_add_test_case "jail_noinet" atf_add_test_case "allowed_peer" atf_add_test_case "allowed_peer_forwarding"home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?69646da6.f507.271aa284>