On 2020-04-21 12:07, Paul Moore wrote:
> If audit_list_rules_send() fails when trying to create a new thread
> to send the rules it also fails to cleanup properly, leaking a
> reference to a net structure.  This patch fixes the error patch and
> renames audit_send_list() to audit_send_list_thread() to better
> match its cousin, audit_send_reply_thread().

Ok, like audit_send_reply(), looks good to me.

> Reported-by: [email protected]
> Signed-off-by: Paul Moore <[email protected]>

Reviewed-by: Richard Guy Briggs <[email protected]>

> ---
>  kernel/audit.c       |    2 +-
>  kernel/audit.h       |    2 +-
>  kernel/auditfilter.c |   16 +++++++---------
>  3 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 66b81358b64f..622c30246d19 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -880,7 +880,7 @@ static int kauditd_thread(void *dummy)
>       return 0;
>  }
>  
> -int audit_send_list(void *_dest)
> +int audit_send_list_thread(void *_dest)
>  {
>       struct audit_netlink_list *dest = _dest;
>       struct sk_buff *skb;
> diff --git a/kernel/audit.h b/kernel/audit.h
> index 2eed4d231624..f0233dc40b17 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -229,7 +229,7 @@ struct audit_netlink_list {
>       struct sk_buff_head q;
>  };
>  
> -int audit_send_list(void *_dest);
> +int audit_send_list_thread(void *_dest);
>  
>  extern int selinux_audit_rule_update(void);
>  
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 026e34da4ace..a10e2997aa6c 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -1161,11 +1161,8 @@ int audit_rule_change(int type, int seq, void *data, 
> size_t datasz)
>   */
>  int audit_list_rules_send(struct sk_buff *request_skb, int seq)
>  {
> -     u32 portid = NETLINK_CB(request_skb).portid;
> -     struct net *net = sock_net(NETLINK_CB(request_skb).sk);
>       struct task_struct *tsk;
>       struct audit_netlink_list *dest;
> -     int err = 0;
>  
>       /* We can't just spew out the rules here because we might fill
>        * the available socket buffer space and deadlock waiting for
> @@ -1173,25 +1170,26 @@ int audit_list_rules_send(struct sk_buff 
> *request_skb, int seq)
>        * happen if we're actually running in the context of auditctl
>        * trying to _send_ the stuff */
>  
> -     dest = kmalloc(sizeof(struct audit_netlink_list), GFP_KERNEL);
> +     dest = kmalloc(sizeof(*dest), GFP_KERNEL);
>       if (!dest)
>               return -ENOMEM;
> -     dest->net = get_net(net);
> -     dest->portid = portid;
> +     dest->net = get_net(sock_net(NETLINK_CB(request_skb).sk));
> +     dest->portid = NETLINK_CB(request_skb).portid;
>       skb_queue_head_init(&dest->q);
>  
>       mutex_lock(&audit_filter_mutex);
>       audit_list_rules(seq, &dest->q);
>       mutex_unlock(&audit_filter_mutex);
>  
> -     tsk = kthread_run(audit_send_list, dest, "audit_send_list");
> +     tsk = kthread_run(audit_send_list_thread, dest, "audit_send_list");
>       if (IS_ERR(tsk)) {
>               skb_queue_purge(&dest->q);
> +             put_net(dest->net);
>               kfree(dest);
> -             err = PTR_ERR(tsk);
> +             return PTR_ERR(tsk);
>       }
>  
> -     return err;
> +     return 0;
>  }
>  
>  int audit_comparator(u32 left, u32 op, u32 right)
> 

- RGB

--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

Reply via email to