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

[Bradley Morgan <brads@mainlining.org>]

On 4 November 2026 20:36:12 GMT, Paul Moore <paul@paul-moore.com> 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?

From what I recall from review, sorry, crappy head. This does fix a *bug*
of some sort, but I don't remember what it fixes, kasan does report the
UAF, and this fix would be a good approach, but just things are messed up
at the moment 

>
>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
>

--- Thanks!