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

[Jérémy Jean]

On 2026-11-04 21:36, Paul Moore wrote:
> On Oct 31, 2026 Bradley Morgan <brads@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
> > Signed-off-by: Jérémy Jean <Jeremy.Jean@xxxxxxxxxxxxxxxxx>
> > ---
> >  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?

Dear Bradley and Paul,

Thanks for the comments. The fix is closely related to the current
code but the main difference is that netlink_trim() does not always
reach its skb_shared() clone path. Moving the clone earlier in the
execution avoids the UAF.

I do have a reproducer, which as Bradley pointed out, triggers the UAF
by indeed taking the early abort path with the delta * 2 < skb->truesize
condition in netlink_trim(). This makes netlink_trim() returns the
original shared skb, and this path does not reach clone of skb a few
lines later in netlink_trim(). In the fix, by calling skb_clone()
explicitely before the call to netlink_unicast(), audit keeps the
original skb, the clone is given to netlink, and the UAF is avoided.

I hope this clarifies.

Regards,
Jérémy