Re: [PATCH] audit: fix UAF in kauditd_send_queue()

[Paul Moore <paul@paul-moore.com>]

On Oct 31, 2026 Bradley Morgan <brads@mainlining.org> wrote:
> 
> kauditd_send_queue() grabs an extra reference to the skb header before
> netlink_unicast() since it may need to retry a failed send. If unicast
> delivery reuses that same header, the receive path stores the
> destination socket in skb->sk without taking a socket reference.
> Consequently, after the receiver consumes the skb and closes the
> socket, the extra reference taken on the skb header makes kauditd
> release a header whose skb->sk points to freed socket storage.
> 
> KASAN reports a UAF as:
> 
>   BUG: KASAN: slab-use-after-free in sock_rfree+0x44/0x140
>   Write of size 4 at addr ffff888004f94140 by task kauditd/31
> 
> Clone the header before netlink_unicast(), then use the clone for
> delivery and keep the original skb only for retries. If cloning fails,
> handle it as -EAGAIN so the send can be retried.
> 
> Fixes: f3d357b09295 ("Audit: save audit_backlog_limit audit messages in case auditd comes back")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> ---
>  kernel/audit.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 9412af9144bc..5e739fa606be 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -790,6 +790,7 @@ static int kauditd_send_queue(struct sock *sk, u32 portid,
>  {
>  	int rc = 0;
>  	struct sk_buff *skb = NULL;
> +	struct sk_buff *skb_to_send;
>  	struct sk_buff *skb_tail;
>  	unsigned int failed = 0;
>  
> @@ -810,9 +811,11 @@ static int kauditd_send_queue(struct sock *sk, u32 portid,
>  		}
>  
>  retry:
> -		/* grab an extra skb reference in case of error */
> -		skb_get(skb);
> -		rc = netlink_unicast(sk, skb, portid, 0);
> +		/* Keep the original header unowned for retries. */
> +		skb_to_send = skb_clone(skb, GFP_KERNEL);

As Bradley pointed out, this appears a little confusing as the existing
approach of skb_get()+netlink_unicast() should end up doing something
very similar to what you are proposing due to netlink_unicast() calling
into netlink_trim() which in this case should always end up doing a
skb_clone().  I'm sure I'm missing something, can you help me understand
why this proposed patch is different and solves the problem?

Also, do you have a reproducer for this?

> +		/* Preserve the connection on a temporary allocation failure. */
> +		rc = skb_to_send ?
> +			netlink_unicast(sk, skb_to_send, portid, 0) : -EAGAIN;
>  		if (rc < 0) {
>  			/* send failed - try a few times unless fatal error */
>  			if (++failed >= retry_limit ||
> @@ -827,7 +830,7 @@ retry:
>  			} else
>  				goto retry;
>  		} else {
> -			/* skb sent - drop the extra reference and continue */
> +			/* skb sent - consume the original and continue */
>  			consume_skb(skb);
>  			failed = 0;
>  		}
> -- 
> 2.47.3

--
paul-moore.com