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 <[email protected]>
I hope it helps!
-Ricardo
