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 <[email protected]>
Cc: [email protected]
Reviewed-by: Ricardo Robaina <[email protected]>
---
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
