auditd(8): register signal handlers interrutibly #36

asomers · 2018-06-28T15:37:50Z

auditd_wait_for_events() relies on read(2) being interrupted by signals,
but it registers signal handlers with signal(3), which sets SA_RESTART.
That breaks asynchronous signal handling. It means that signals don't
actually get handled until after an audit(8) trigger is received.
Symptoms include:

Sending SIGTERM to auditd doesn't kill it right away; you must send
SIGTERM and then send a trigger with auditon(2).
Same with SIGHUP
Zombie child processes don't get reaped until auditd receives a trigger
sent by auditon. This includes children created by expiring audit trails
at auditd startup.

Fix by using sigaction(2) instead of signal(3).

Fixes #34

cemeyer

LGTM modulo style nitpicking :-)

cemeyer · 2018-06-28T18:18:50Z

bin/auditd/auditd.c

 		auditd_log_err(
 		    "Could not set signal handler for SIGTERM");
 		fail_exit();
 	}
-	if (signal(SIGCHLD, auditd_relay_signal) == SIG_ERR) {
+	if (sigaction(SIGCHLD, &action, NULL)) {


Implicit treatment of integer return value as bool; instead, compare against 0? E.g., if (sigaction() != 0) or if (sigaction() < 0). (Same below.)

cemeyer · 2018-06-28T18:24:11Z

The revision looks good to me, but please squash the two commits together for merge. (I think you can just git rebase -i HEAD^^ and then git push --force.)

auditd_wait_for_events() relies on read(2) being interrupted by signals, but it registers signal handlers with signal(3), which sets SA_RESTART. That breaks asynchronous signal handling. It means that signals don't actually get handled until after an audit(8) trigger is received. Symptoms include: * Sending SIGTERM to auditd doesn't kill it right away; you must send SIGTERM and then send a trigger with auditon(2). * Same with SIGHUP * Zombie child processes don't get reaped until auditd receives a trigger sent by auditon. This includes children created by expiring audit trails at auditd startup. Fix by using sigaction(2) instead of signal(3). Fixes openbsm#34

0mp · 2018-06-28T21:16:37Z

@cemeyer BTW, I think that you can squash them while merging as well using the GitHub interface.

auditd_wait_for_events() relies on read(2) being interrupted by signals, but it registers signal handlers with signal(3), which sets SA_RESTART. That breaks asynchronous signal handling. It means that signals don't actually get handled until after an audit(8) trigger is received. Symptoms include: * Sending SIGTERM to auditd doesn't kill it right away; you must send SIGTERM and then send a trigger with auditon(2). * Same with SIGHUP * Zombie child processes don't get reaped until auditd receives a trigger sent by auditon. This includes children created by expiring audit trails at auditd startup. Fix by using sigaction(2) instead of signal(3). Cherry pick openbsm/openbsm@d060887 PR: 229381 Reviewed by: cem Obtained from: OpenBSM MFC after: 2 weeks Differential Revision: openbsm/openbsm#36 git-svn-id: svn+ssh://svn.freebsd.org/base/head@335899 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f

auditd_wait_for_events() relies on read(2) being interrupted by signals, but it registers signal handlers with signal(3), which sets SA_RESTART. That breaks asynchronous signal handling. It means that signals don't actually get handled until after an audit(8) trigger is received. Symptoms include: * Sending SIGTERM to auditd doesn't kill it right away; you must send SIGTERM and then send a trigger with auditon(2). * Same with SIGHUP * Zombie child processes don't get reaped until auditd receives a trigger sent by auditon. This includes children created by expiring audit trails at auditd startup. Fix by using sigaction(2) instead of signal(3). Cherry pick openbsm/openbsm@d060887 PR: 229381 Reviewed by: cem Obtained from: OpenBSM MFC after: 2 weeks Differential Revision: openbsm/openbsm#36

auditd_wait_for_events() relies on read(2) being interrupted by signals, but it registers signal handlers with signal(3), which sets SA_RESTART. That breaks asynchronous signal handling. It means that signals don't actually get handled until after an audit(8) trigger is received. Symptoms include: * Sending SIGTERM to auditd doesn't kill it right away; you must send SIGTERM and then send a trigger with auditon(2). * Same with SIGHUP * Zombie child processes don't get reaped until auditd receives a trigger sent by auditon. This includes children created by expiring audit trails at auditd startup. Fix by using sigaction(2) instead of signal(3). Cherry pick openbsm/openbsm@d060887 PR: 229381 Reviewed by: cem Obtained from: OpenBSM MFC after: 2 weeks Differential Revision: openbsm/openbsm#36 git-svn-id: https://svn.freebsd.org/base/head@335899 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f

auditd_wait_for_events() relies on read(2) being interrupted by signals, but it registers signal handlers with signal(3), which sets SA_RESTART. That breaks asynchronous signal handling. It means that signals don't actually get handled until after an audit(8) trigger is received. Symptoms include: * Sending SIGTERM to auditd doesn't kill it right away; you must send SIGTERM and then send a trigger with auditon(2). * Same with SIGHUP * Zombie child processes don't get reaped until auditd receives a trigger sent by auditon. This includes children created by expiring audit trails at auditd startup. Fix by using sigaction(2) instead of signal(3). Cherry pick openbsm/openbsm@d060887 PR: 229381 Reviewed by: cem Obtained from: OpenBSM MFC after: 2 weeks Differential Revision: openbsm/openbsm#36 git-svn-id: svn+ssh://svn.freebsd.org/base/head@335899 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f

auditd(8): register signal handlers interrutibly auditd_wait_for_events() relies on read(2) being interrupted by signals, but it registers signal handlers with signal(3), which sets SA_RESTART. That breaks asynchronous signal handling. It means that signals don't actually get handled until after an audit(8) trigger is received. Symptoms include: * Sending SIGTERM to auditd doesn't kill it right away; you must send SIGTERM and then send a trigger with auditon(2). * Same with SIGHUP * Zombie child processes don't get reaped until auditd receives a trigger sent by auditon. This includes children created by expiring audit trails at auditd startup. Fix by using sigaction(2) instead of signal(3). Cherry pick openbsm/openbsm@d060887 PR: 229381 Reviewed by: cem Obtained from: OpenBSM Differential Revision: openbsm/openbsm#36

auditd(8): register signal handlers interrutibly auditd_wait_for_events() relies on read(2) being interrupted by signals, but it registers signal handlers with signal(3), which sets SA_RESTART. That breaks asynchronous signal handling. It means that signals don't actually get handled until after an audit(8) trigger is received. Symptoms include: * Sending SIGTERM to auditd doesn't kill it right away; you must send SIGTERM and then send a trigger with auditon(2). * Same with SIGHUP * Zombie child processes don't get reaped until auditd receives a trigger sent by auditon. This includes children created by expiring audit trails at auditd startup. Fix by using sigaction(2) instead of signal(3). Cherry pick openbsm/openbsm@d060887 PR: 229381 Reviewed by: cem Obtained from: OpenBSM Differential Revision: openbsm/openbsm#36 git-svn-id: https://svn.freebsd.org/base/stable/10@337257 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f

auditd(8): register signal handlers interrutibly auditd_wait_for_events() relies on read(2) being interrupted by signals, but it registers signal handlers with signal(3), which sets SA_RESTART. That breaks asynchronous signal handling. It means that signals don't actually get handled until after an audit(8) trigger is received. Symptoms include: * Sending SIGTERM to auditd doesn't kill it right away; you must send SIGTERM and then send a trigger with auditon(2). * Same with SIGHUP * Zombie child processes don't get reaped until auditd receives a trigger sent by auditon. This includes children created by expiring audit trails at auditd startup. Fix by using sigaction(2) instead of signal(3). Cherry pick openbsm/openbsm@d060887 PR: 229381 Reviewed by: cem Obtained from: OpenBSM Differential Revision: openbsm/openbsm#36 git-svn-id: https://svn.freebsd.org/base/stable/11@337241 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f

auditd_wait_for_events() relies on read(2) being interrupted by signals, but it registers signal handlers with signal(3), which sets SA_RESTART. That breaks asynchronous signal handling. It means that signals don't actually get handled until after an audit(8) trigger is received. Symptoms include: * Sending SIGTERM to auditd doesn't kill it right away; you must send SIGTERM and then send a trigger with auditon(2). * Same with SIGHUP * Zombie child processes don't get reaped until auditd receives a trigger sent by auditon. This includes children created by expiring audit trails at auditd startup. Fix by using sigaction(2) instead of signal(3). Cherry pick openbsm/openbsm@d060887 PR: 229381 Reviewed by: cem Obtained from: OpenBSM MFC after: 2 weeks Differential Revision: openbsm/openbsm#36

auditd(8): register signal handlers interrutibly auditd_wait_for_events() relies on read(2) being interrupted by signals, but it registers signal handlers with signal(3), which sets SA_RESTART. That breaks asynchronous signal handling. It means that signals don't actually get handled until after an audit(8) trigger is received. Symptoms include: * Sending SIGTERM to auditd doesn't kill it right away; you must send SIGTERM and then send a trigger with auditon(2). * Same with SIGHUP * Zombie child processes don't get reaped until auditd receives a trigger sent by auditon. This includes children created by expiring audit trails at auditd startup. Fix by using sigaction(2) instead of signal(3). Cherry pick openbsm/openbsm@d060887 PR: 229381 Reviewed by: cem Obtained from: OpenBSM Differential Revision: openbsm/openbsm#36

cemeyer reviewed Jun 28, 2018

View reviewed changes

asomers force-pushed the issue_34 branch from 5920ee2 to f0bf4e0 Compare June 28, 2018 18:25

cemeyer merged commit d060887 into openbsm:master Jul 3, 2018

asomers deleted the issue_34 branch July 3, 2018 00:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

auditd(8): register signal handlers interrutibly #36

auditd(8): register signal handlers interrutibly #36

asomers commented Jun 28, 2018

cemeyer left a comment

cemeyer Jun 28, 2018

cemeyer commented Jun 28, 2018 •

edited

Loading

0mp commented Jun 28, 2018

auditd(8): register signal handlers interrutibly #36

auditd(8): register signal handlers interrutibly #36

Conversation

asomers commented Jun 28, 2018

cemeyer left a comment

Choose a reason for hiding this comment

cemeyer Jun 28, 2018

Choose a reason for hiding this comment

cemeyer commented Jun 28, 2018 • edited Loading

0mp commented Jun 28, 2018

cemeyer commented Jun 28, 2018 •

edited

Loading