[PATCH v2] audit: Allow auditd to set pid to 0 to end auditing
Steve Grubb
sgrubb at redhat.com
Wed Oct 18 22:49:13 UTC 2017
On Wednesday, October 18, 2017 6:31:47 PM EDT Paul Moore wrote:
> > auditd_pid = auditd_pid_vnr();
> > - /* only the current auditd can unregister itself
> > */
> > - if ((!new_pid) && (new_pid != auditd_pid)) {
> > - audit_log_config_change("audit_pid",
> > new_pid, -
> > auditd_pid, 0); - return -EACCES;
> > - }
> > - /* replacing a healthy auditd is not allowed */
> > - if (auditd_pid && new_pid) {
> > - audit_log_config_change("audit_pid",
> > new_pid, -
> > auditd_pid, 0); - return -EEXIST;
> > + if (auditd_pid) {
> > + /* replacing a healthy auditd is not
> > allowed */ + if (new_pid) {
> > +
> > audit_log_config_change("audit_pid", +
> > new_pid, auditd_pid, 0); +
> > return -EEXIST;
> > + }
> > + /* only current auditd can unregister
> > itself */ + if (pid_vnr(req_pid) !=
> > auditd_pid) { +
> > audit_log_config_change("audit_pid", +
> > new_pid, auditd_pid, 0); +
> > return -EACCES;
> > + }
>
> I realize that you reordered the checks to simplify the conditionals,
> but you did reorder the checks ... I'm thinking out loud right now
> trying to figure out if that really matters ... probably not,
> especially since the checks were broken anyway ... and you need
> CAP_AUDIT_CONTROL to even get this far ... we're probably okay.
Yes when refactoring as you suggested I realized that we can also remove some
checks for new_pid == 0 because if its not, it takes the first "if" which
returns. Therefore new_pid is guaranteed to be 0 and no check for that is
needed. :-)
-Steve
More information about the Linux-audit
mailing list