There is a potential data race in zd_mac_tx_to_dev(). For example, it is
possible for filter_ack() to clear the ack_wait_queue right after
zd_mac_tx_to_dev() checks that queue has more than 50 elements, but before
it dequeues any skb. This results in skb_dequeue() returns NULL and the
pointer is dereferenced in zd_mac_tx_status().
In order to avoid potential data races and leading dereference of a NULL
pointer, acquire the queue lock before any work with the queue is done and
replace all skb_* calls with their lockless version.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: 459c51ad6e1f ("zd1211rw: port to mac80211")
Signed-off-by: Daniil Dulov <d.dulov@xxxxxxxxxx>
---
drivers/net/wireless/zydas/zd1211rw/zd_mac.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/zydas/zd1211rw/zd_mac.c b/drivers/net/wireless/zydas/zd1211rw/zd_mac.c
index 9653dbaac3c0..e7e0d1b7b9ab 100644
--- a/drivers/net/wireless/zydas/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zydas/zd1211rw/zd_mac.c
@@ -568,6 +568,7 @@ void zd_mac_tx_to_dev(struct sk_buff *skb, int error)
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
struct ieee80211_hw *hw = info->rate_driver_data[0];
struct zd_mac *mac = zd_hw_mac(hw);
+ unsigned long flags;
ieee80211_tx_info_clear_status(info);
@@ -581,13 +582,17 @@ void zd_mac_tx_to_dev(struct sk_buff *skb, int error)
} else {
struct sk_buff_head *q = &mac->ack_wait_queue;
- skb_queue_tail(q, skb);
+ spin_lock_irqsave(&q->lock, flags);
+
+ __skb_queue_tail(q, skb);
while (skb_queue_len(q) > ZD_MAC_MAX_ACK_WAITERS) {
- zd_mac_tx_status(hw, skb_dequeue(q),
+ zd_mac_tx_status(hw, __skb_dequeue(q),
mac->ack_pending ? mac->ack_signal : 0,
NULL);
mac->ack_pending = 0;
}
+
+ spin_unlock_irqrestore(&q->lock, flags);
}
}
--
2.34.1
