When ath9k was switched over to use the mac80211 intermediate queues,
node cleanup now drains the mac80211 queues. However, this call path is
not protected by rcu_read_lock() as it was previously entirely internal
to the driver which uses its own locking.
This leads to a possible rcu_dereference() without holding
rcu_read_lock(); but only if a station is cleaned up while having
packets queued on the TXQ. Fix this by adding the rcu_read_lock() to the
caller in ath9k.
Fixes: 50f08edf9809 ("ath9k: Switch to using mac80211 intermediate software queues.")
Cc: [email protected]
Reported-by: Ben Greear <[email protected]>
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
---
drivers/net/wireless/ath/ath9k/xmit.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 396bf05c6bf6..d8b041f48ca8 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -2892,6 +2892,8 @@ void ath_tx_node_cleanup(struct ath_softc *sc, struct ath_node *an)
struct ath_txq *txq;
int tidno;
+ rcu_read_lock();
+
for (tidno = 0; tidno < IEEE80211_NUM_TIDS; tidno++) {
tid = ath_node_to_tid(an, tidno);
txq = tid->txq;
@@ -2909,6 +2911,8 @@ void ath_tx_node_cleanup(struct ath_softc *sc, struct ath_node *an)
if (!an->sta)
break; /* just one multicast ath_atx_tid */
}
+
+ rcu_read_unlock();
}
#ifdef CONFIG_ATH9K_TX99
--
2.16.0
Toke Høiland-Jørgensen wrote:
> When ath9k was switched over to use the mac80211 intermediate queues,
> node cleanup now drains the mac80211 queues. However, this call path is
> not protected by rcu_read_lock() as it was previously entirely internal
> to the driver which uses its own locking.
>
> This leads to a possible rcu_dereference() without holding
> rcu_read_lock(); but only if a station is cleaned up while having
> packets queued on the TXQ. Fix this by adding the rcu_read_lock() to the
> caller in ath9k.
>
> Fixes: 50f08edf9809 ("ath9k: Switch to using mac80211 intermediate software queues.")
> Cc: [email protected]
> Reported-by: Ben Greear <[email protected]>
> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>
Patch applied to ath-next branch of ath.git, thanks.
182b19171098 ath9k: Protect queue draining by rcu_read_lock()
--
https://patchwork.kernel.org/patch/10196453/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 02/02/2018 02:36 AM, Toke Høiland-Jørgensen wrote:
> When ath9k was switched over to use the mac80211 intermediate queues,
> node cleanup now drains the mac80211 queues. However, this call path is
> not protected by rcu_read_lock() as it was previously entirely internal
> to the driver which uses its own locking.
As far as I can tell, this is not currently in Linus' tree.
Was this dropped on purpose?
Thanks,
Ben
>
> This leads to a possible rcu_dereference() without holding
> rcu_read_lock(); but only if a station is cleaned up while having
> packets queued on the TXQ. Fix this by adding the rcu_read_lock() to the
> caller in ath9k.
>
> Fixes: 50f08edf9809 ("ath9k: Switch to using mac80211 intermediate software queues.")
> Cc: [email protected]
> Reported-by: Ben Greear <[email protected]>
> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
> ---
> drivers/net/wireless/ath/ath9k/xmit.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index 396bf05c6bf6..d8b041f48ca8 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -2892,6 +2892,8 @@ void ath_tx_node_cleanup(struct ath_softc *sc, struct ath_node *an)
> struct ath_txq *txq;
> int tidno;
>
> + rcu_read_lock();
> +
> for (tidno = 0; tidno < IEEE80211_NUM_TIDS; tidno++) {
> tid = ath_node_to_tid(an, tidno);
> txq = tid->txq;
> @@ -2909,6 +2911,8 @@ void ath_tx_node_cleanup(struct ath_softc *sc, struct ath_node *an)
> if (!an->sta)
> break; /* just one multicast ath_atx_tid */
> }
> +
> + rcu_read_unlock();
> }
>
> #ifdef CONFIG_ATH9K_TX99
>
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On 24 March 2018 00:00:39 GMT, Ben Greear <greearb@candelatech=2Ecom> wrot=
e:
>On 02/02/2018 02:36 AM, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>> When ath9k was switched over to use the mac80211 intermediate queues,
>> node cleanup now drains the mac80211 queues=2E However, this call path
>is
>> not protected by rcu_read_lock() as it was previously entirely
>internal
>> to the driver which uses its own locking=2E
>
>As far as I can tell, this is not currently in Linus' tree=2E
>
>Was this dropped on purpose?
It was merged quite recently, but hasn't propagated yet:
https://git=2Ekernel=2Eorg/pub/scm/linux/kernel/git/kvalo/ath=2Egit/commit=
/?h=3Dath-next
-Toke