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 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit