The branch main has been updated by jfree:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=60ae4e52f33e3c67720b68a29e35f6c114a3386c

commit 60ae4e52f33e3c67720b68a29e35f6c114a3386c
Author:     Jake Freeland <[email protected]>
AuthorDate: 2025-12-22 06:05:37 +0000
Commit:     Jake Freeland <[email protected]>
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"

Reply via email to