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

["Jérémy Jean" <jeremy.jean@oss.cyber.gouv.fr>]

On 2026-11-04 21:36, Paul Moore wrote:
> 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?

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