mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] audit: Fix data race in audit_log_start()
@ 2026-06-17 13:00 Chi Wang
  2026-06-18 13:40 ` Ricardo Robaina
  2026-06-19  7:42 ` [PATCH v2] audit: Fix data races of skb_queue_len() readers on audit_queue Chi Wang
  0 siblings, 2 replies; 4+ messages in thread
From: Chi Wang @ 2026-06-17 13:00 UTC (permalink / raw)
  To: Paul Moore, Eric Paris; +Cc: audit, linux-kernel, stable, Chi Wang

audit_log_start() reads audit_queue.qlen via skb_queue_len() without
holding the queue lock or using READ_ONCE(), while kauditd writes to
this field via the skb_dequeue() → __skb_unlink() path with WRITE_ONCE()
protected by a spinlock. This constitutes a data race.

KCSAN reports the following conflicting access pattern:
==================================================================
BUG: KCSAN: data-race in audit_log_start / skb_dequeue

write (marked) to 0xffffffff8512ee20 of 4 bytes by task 661 on cpu 57:
 skb_dequeue+0x70/0xf0
 kauditd_send_queue+0x71/0x220
 kauditd_thread+0x1cb/0x430
 kthread+0x1c2/0x210
 ret_from_fork+0x162/0x1a0
 ret_from_fork_asm+0x1a/0x30

read to 0xffffffff8512ee20 of 4 bytes by task 36586 on cpu 1:
 audit_log_start+0x2a0/0x6b0
 audit_core_dumps+0x64/0xa0
 do_coredump+0x14b/0x1260
 get_signal+0xeb2/0xf70
 arch_do_signal_or_restart+0x41/0x170
 exit_to_user_mode_loop+0xa2/0x1c0
 do_syscall_64+0x1a3/0x1c0
 entry_SYSCALL_64_after_hwframe+0x76/0xe0

value changed: 0x00000001 -> 0x00000000
==================================================================

Resolve the race by switching to lockless helper skb_queue_len_lockless(),
which internally uses READ_ONCE() and properly pairs with the WRITE_ONCE()
write accesses already present on the writer side.

Fixes: 3197542482df ("audit: rework audit_log_start()")
Signed-off-by: Chi Wang <wangchi@kylinos.cn>
Cc: stable@vger.kernel.org
---
 kernel/audit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 34dc7cb246ff..e4c095017302 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1933,7 +1933,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
 		long stime = audit_backlog_wait_time;

 		while (audit_backlog_limit &&
-		       (skb_queue_len(&audit_queue) > audit_backlog_limit)) {
+			(skb_queue_len_lockless(&audit_queue) > audit_backlog_limit)) {
 			/* wake kauditd to try and flush the queue */
 			wake_up_interruptible(&kauditd_wait);

@@ -1953,7 +1953,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
 			} else {
 				if (audit_rate_check() && printk_ratelimit())
 					pr_warn("audit_backlog=%d > audit_backlog_limit=%d\n",
-						skb_queue_len(&audit_queue),
+						skb_queue_len_lockless(&audit_queue),
 						audit_backlog_limit);
 				audit_log_lost("backlog limit exceeded");
 				return NULL;
--
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] audit: Fix data race in audit_log_start()
  2026-06-17 13:00 [PATCH] audit: Fix data race in audit_log_start() Chi Wang
@ 2026-06-18 13:40 ` Ricardo Robaina
  2026-06-19  7:42 ` [PATCH v2] audit: Fix data races of skb_queue_len() readers on audit_queue Chi Wang
  1 sibling, 0 replies; 4+ messages in thread
From: Ricardo Robaina @ 2026-06-18 13:40 UTC (permalink / raw)
  To: wangchi; +Cc: paul, eparis, audit, linux-kernel, Ricardo Robaina

Hi Chi,

Good catch, the patch looks good to me.

However, I found three additional instances of the same pattern that
should be included in this patch for completeness:

  1. Line 953 in kauditd_thread():
     wait_event_freezable(kauditd_wait,
                          (skb_queue_len(&audit_queue) ? 1 : 0));

  2. Line 1286 in audit_receive_msg() (AUDIT_GET handler):
     s.backlog = skb_queue_len(&audit_queue);

  3. Line 1630 in audit_receive():
     if (audit_backlog_limit &&
         (skb_queue_len(&audit_queue) > audit_backlog_limit)) {

All three read audit_queue.qlen without synchronization while the queue
is concurrently modified.

diff --git a/kernel/audit.c b/kernel/audit.c
index dcc657d35776..c074c3717946 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -950,7 +950,7 @@ static int kauditd_thread(void *dummy)
                 *       do the multicast send and rotate records from the
                 *       main queue to the retry/hold queues */
                wait_event_freezable(kauditd_wait,
-                                    (skb_queue_len(&audit_queue) ? 1 : 0));
+                                    (skb_queue_len_lockless(&audit_queue) ? 1 : 0));
        }
 
        return 0;
@@ -1283,7 +1283,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
                s.rate_limit               = audit_rate_limit;
                s.backlog_limit            = audit_backlog_limit;
                s.lost                     = atomic_read(&audit_lost);
-               s.backlog                  = skb_queue_len(&audit_queue);
+               s.backlog                  = skb_queue_len_lockless(&audit_queue);
                s.feature_bitmap           = AUDIT_FEATURE_BITMAP_ALL;
                s.backlog_wait_time        = audit_backlog_wait_time;
                s.backlog_wait_time_actual = atomic_read(&audit_backlog_wait_time_actual);
@@ -1627,7 +1627,7 @@ static void audit_receive(struct sk_buff *skb)
 
        /* can't block with the ctrl lock, so penalize the sender now */
        if (audit_backlog_limit &&
-           (skb_queue_len(&audit_queue) > audit_backlog_limit)) {
+           (skb_queue_len_lockless(&audit_queue) > audit_backlog_limit)) {
                DECLARE_WAITQUEUE(wait, current);
 
                /* wake kauditd to try and flush the queue */

Reviewed-by: Ricardo Robaina <rrobaina@redhat.com>

I hope it helps!
-Ricardo


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] audit: Fix data races of skb_queue_len() readers on audit_queue
  2026-06-17 13:00 [PATCH] audit: Fix data race in audit_log_start() Chi Wang
  2026-06-18 13:40 ` Ricardo Robaina
@ 2026-06-19  7:42 ` Chi Wang
  2026-06-30 20:15   ` Paul Moore
  1 sibling, 1 reply; 4+ messages in thread
From: Chi Wang @ 2026-06-19  7:42 UTC (permalink / raw)
  To: Paul Moore, Eric Paris
  Cc: audit, linux-kernel, stable, Ricardo Robaina, Chi Wang

Multiple readers access audit_queue.qlen via skb_queue_len() without
holding the queue lock or using READ_ONCE(), while kauditd writes to
this field via the skb_dequeue() → __skb_unlink() path with WRITE_ONCE()
protected by a spinlock. This constitutes data races.

All affected skb_queue_len(&audit_queue) call sites:
  - kauditd_thread() wait_event_freezable() condition
  - audit_receive_msg() AUDIT_GET handler (s.backlog assignment)
  - audit_receive() backlog check
  - audit_log_start() backlog check and pr_warn()

KCSAN reports the following conflicting access pattern (one example):
==================================================================
BUG: KCSAN: data-race in audit_log_start / skb_dequeue

write (marked) to 0xffffffff8512ee20 of 4 bytes by task 661 on cpu 57:
 skb_dequeue+0x70/0xf0
 kauditd_send_queue+0x71/0x220
 kauditd_thread+0x1cb/0x430
 kthread+0x1c2/0x210
 ret_from_fork+0x162/0x1a0
 ret_from_fork_asm+0x1a/0x30

read to 0xffffffff8512ee20 of 4 bytes by task 36586 on cpu 1:
 audit_log_start+0x2a0/0x6b0
 audit_core_dumps+0x64/0xa0
 do_coredump+0x14b/0x1260
 get_signal+0xeb2/0xf70
 arch_do_signal_or_restart+0x41/0x170
 exit_to_user_mode_loop+0xa2/0x1c0
 do_syscall_64+0x1a3/0x1c0
 entry_SYSCALL_64_after_hwframe+0x76/0xe0

value changed: 0x00000001 -> 0x00000000
==================================================================

Resolve the race by switching to lockless helper skb_queue_len_lockless(),
which internally uses READ_ONCE() and properly pairs with the WRITE_ONCE()
write accesses already present on the writer side.

Fixes: 3197542482df ("audit: rework audit_log_start()")
Signed-off-by: Chi Wang <wangchi@kylinos.cn>
Cc: stable@vger.kernel.org
Reviewed-by: Ricardo Robaina <rrobaina@redhat.com>
---
 kernel/audit.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 34dc7cb246ff..cf385393d1ed 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -950,7 +950,7 @@ static int kauditd_thread(void *dummy)
 		 *       do the multicast send and rotate records from the
 		 *       main queue to the retry/hold queues */
 		wait_event_freezable(kauditd_wait,
-				     (skb_queue_len(&audit_queue) ? 1 : 0));
+				     (skb_queue_len_lockless(&audit_queue) ? 1 : 0));
 	}

 	return 0;
@@ -1283,7 +1283,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 		s.rate_limit		   = audit_rate_limit;
 		s.backlog_limit		   = audit_backlog_limit;
 		s.lost			   = atomic_read(&audit_lost);
-		s.backlog		   = skb_queue_len(&audit_queue);
+		s.backlog		   = skb_queue_len_lockless(&audit_queue);
 		s.feature_bitmap	   = AUDIT_FEATURE_BITMAP_ALL;
 		s.backlog_wait_time	   = audit_backlog_wait_time;
 		s.backlog_wait_time_actual = atomic_read(&audit_backlog_wait_time_actual);
@@ -1627,7 +1627,7 @@ static void audit_receive(struct sk_buff *skb)

 	/* can't block with the ctrl lock, so penalize the sender now */
 	if (audit_backlog_limit &&
-	    (skb_queue_len(&audit_queue) > audit_backlog_limit)) {
+	    (skb_queue_len_lockless(&audit_queue) > audit_backlog_limit)) {
 		DECLARE_WAITQUEUE(wait, current);

 		/* wake kauditd to try and flush the queue */
@@ -1933,7 +1933,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
 		long stime = audit_backlog_wait_time;

 		while (audit_backlog_limit &&
-		       (skb_queue_len(&audit_queue) > audit_backlog_limit)) {
+			(skb_queue_len_lockless(&audit_queue) > audit_backlog_limit)) {
 			/* wake kauditd to try and flush the queue */
 			wake_up_interruptible(&kauditd_wait);

@@ -1953,7 +1953,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
 			} else {
 				if (audit_rate_check() && printk_ratelimit())
 					pr_warn("audit_backlog=%d > audit_backlog_limit=%d\n",
-						skb_queue_len(&audit_queue),
+						skb_queue_len_lockless(&audit_queue),
 						audit_backlog_limit);
 				audit_log_lost("backlog limit exceeded");
 				return NULL;
--
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] audit: Fix data races of skb_queue_len() readers on  audit_queue
  2026-06-19  7:42 ` [PATCH v2] audit: Fix data races of skb_queue_len() readers on audit_queue Chi Wang
@ 2026-06-30 20:15   ` Paul Moore
  0 siblings, 0 replies; 4+ messages in thread
From: Paul Moore @ 2026-06-30 20:15 UTC (permalink / raw)
  To: Chi Wang, Eric Paris
  Cc: audit, linux-kernel, stable, Ricardo Robaina, Chi Wang

On Jun 19, 2026 Chi Wang <wangchi@kylinos.cn> wrote:
> 
> Multiple readers access audit_queue.qlen via skb_queue_len() without
> holding the queue lock or using READ_ONCE(), while kauditd writes to
> this field via the skb_dequeue() → __skb_unlink() path with WRITE_ONCE()
> protected by a spinlock. This constitutes data races.
> 
> All affected skb_queue_len(&audit_queue) call sites:
>   - kauditd_thread() wait_event_freezable() condition
>   - audit_receive_msg() AUDIT_GET handler (s.backlog assignment)
>   - audit_receive() backlog check
>   - audit_log_start() backlog check and pr_warn()
> 
> KCSAN reports the following conflicting access pattern (one example):
> ==================================================================
> BUG: KCSAN: data-race in audit_log_start / skb_dequeue
> 
> write (marked) to 0xffffffff8512ee20 of 4 bytes by task 661 on cpu 57:
>  skb_dequeue+0x70/0xf0
>  kauditd_send_queue+0x71/0x220
>  kauditd_thread+0x1cb/0x430
>  kthread+0x1c2/0x210
>  ret_from_fork+0x162/0x1a0
>  ret_from_fork_asm+0x1a/0x30
> 
> read to 0xffffffff8512ee20 of 4 bytes by task 36586 on cpu 1:
>  audit_log_start+0x2a0/0x6b0
>  audit_core_dumps+0x64/0xa0
>  do_coredump+0x14b/0x1260
>  get_signal+0xeb2/0xf70
>  arch_do_signal_or_restart+0x41/0x170
>  exit_to_user_mode_loop+0xa2/0x1c0
>  do_syscall_64+0x1a3/0x1c0
>  entry_SYSCALL_64_after_hwframe+0x76/0xe0
> 
> value changed: 0x00000001 -> 0x00000000
> ==================================================================
> 
> Resolve the race by switching to lockless helper skb_queue_len_lockless(),
> which internally uses READ_ONCE() and properly pairs with the WRITE_ONCE()
> write accesses already present on the writer side.
> 
> Fixes: 3197542482df ("audit: rework audit_log_start()")
> Signed-off-by: Chi Wang <wangchi@kylinos.cn>
> Cc: stable@vger.kernel.org
> Reviewed-by: Ricardo Robaina <rrobaina@redhat.com>
> ---
>  kernel/audit.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Merged into stable-7.2, thanks.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-30 20:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-17 13:00 [PATCH] audit: Fix data race in audit_log_start() Chi Wang
2026-06-18 13:40 ` Ricardo Robaina
2026-06-19  7:42 ` [PATCH v2] audit: Fix data races of skb_queue_len() readers on audit_queue Chi Wang
2026-06-30 20:15   ` Paul Moore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®