mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2026-09-22 09:34:56 +02:00
audit: Fix data races of skb_queue_len() readers on audit_queue
commitc9a71daaecupstream. 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. Cc: stable@vger.kernel.org Fixes:3197542482("audit: rework audit_log_start()") Signed-off-by: Chi Wang <wangchi@kylinos.cn> Reviewed-by: Ricardo Robaina <rrobaina@redhat.com> [PM: line length tweak] Signed-off-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
committed by
Greg Kroah-Hartman
parent
e8417353cb
commit
fe997a84a3
+5
-5
@@ -946,7 +946,7 @@ main_queue:
|
||||
* 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;
|
||||
@@ -1279,7 +1279,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);
|
||||
@@ -1622,7 +1622,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 */
|
||||
@@ -1928,7 +1928,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);
|
||||
|
||||
@@ -1948,7 +1948,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;
|
||||
|
||||
Reference in New Issue
Block a user