[PATCH] audit: fix UAF in kauditd_send_queue()

[PATCH] audit: fix UAF in kauditd_send_queue()

[Jérémy Jean]

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);
+		/* 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

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

[Bradley Morgan]

On 31 October 2026 21:12:59 GMT, "Jérémy Jean"
<Jeremy.Jean@oss.cyber.gouv.fr> 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

What I cant work out from the changelog is where netlink_trim() went.
With users == 2 it usually clones for you anyway (skb_shared() is just
users != 1) and then the receiver owns a different struct, so this
couldnt happen. Unless it took the early return:

	if (is_vmalloc_addr(skb->head) || delta * 2 < skb->truesize)
		return skb;

Which one was it? If its the delta side then its the tightly filled
records that trip this, which explains 18 years of silence pretty well.
It also tells us what this costs, where trim would have cloned anyway
you just moved the clone, otherwise you pay one small slab alloc per
record.

>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. */


A header isnt a skb, and the comment you deleted said skb. Also this
line is where the why belongs, since the why is the bug itself. e.g,

		/* netlink remembers the sock in skb->sk with no ref,
		 * so send a clone and keep the original clean
		 */


>+		skb_to_send = skb_clone(skb, GFP_KERNEL);
>+		/* Preserve the connection on a temporary allocation failure. */

Well, this one doesnt preserve the connection.. -EAGAIN falls into the
same bail branch which does sk = NULL and then squashes rc to 0, so
once the alloc failures hit retry_limit the connection is gone anyway.

It also eats a slot of ++failed for what is really just an ENOMEM and
not a send failure. Might be fine! but the comment says the opposite of
the code, can you make it match?

>+		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;
> 		}
>

--- Thanks!

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

[Paul Moore]

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

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

[Bradley Morgan]

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!