[PATCH 1/1] audit: Allow auditd to set pid to 0 to end auditing

[Paul Moore]

On Fri, Oct 13, 2017 at 3:53 PM, Steve Grubb <sgrubb at redhat.com> wrote:
> The API to end auditing has historically been for auditd to set the
> pid to 0. This patch restores that functionality.
>
> Signed-off-by: sgrubb <sgrubb at redhat.com>
> ---
>  kernel/audit.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 6dd556931739..1baabc9539b4 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1197,8 +1197,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>                         pid_t auditd_pid;
>                         struct pid *req_pid = task_tgid(current);
>
> -                       /* sanity check - PID values must match */
> -                       if (new_pid != pid_vnr(req_pid))
> +                       /* Sanity check - PID values must match. A 0
> +                        * pid is how auditd normally ends auditing. */
> +                       if (new_pid && (new_pid != pid_vnr(req_pid)))
>                                 return -EINVAL;
>
>                         /* test the auditd connection */
> @@ -1206,7 +1207,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>
>                         auditd_pid = auditd_pid_vnr();
>                         /* only the current auditd can unregister itself */
> -                       if ((!new_pid) && (new_pid != auditd_pid)) {
> +                       if (new_pid && auditd_pid && (new_pid != auditd_pid)) {

The existing code is definitely not correct, but it looks like your
proposed change is not correct either: the unnegated new_pid check at
the start of the conditional is always going to be false in the
disconnect case (new_pid == 0).  I think the correct fix is something
like below ...

  if (auditd_pid && (!new_pid) && (pid_vnr(req_pid) != auditd_pid))

... and for bonus points, you could probably combine things a bit with
the no-replace check driectly below that code for soemthing like this
...

  if (auditd_pid) {
          if ((!new_pid) && (pid_vnr(req_pid) != auditd_pid)) {
                  /* blah blah, you can't reset someone else's connection */
          }
          if (new_pid) {
                  /* blah blah, you can't replace a healthy connection */
          }
  }

-- 
paul moore
www.paul-moore.com