[PATCH] ath9k: fix beacon resource related race condition

[PATCH] ath9k: fix beacon resource related race condition

[Rajkumar Manoharan]

While releasing beacon slot, it can be accessed by beacon tasklet.
To avoid concurrency, beacon alert is disabled before
freeing beacon resource and it will be enabled again in the
persence of other beaconing mode interfaces.

Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
---
 drivers/net/wireless/ath/ath9k/main.c |   21 +++++++--------------
 1 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 7acd6b0..eabde4d 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1435,8 +1435,6 @@ static void ath9k_remove_interface(struct ieee80211_hw *hw,
 	struct ath_softc *sc = aphy->sc;
 	struct ath_common *common = ath9k_hw_common(sc->sc_ah);
 	struct ath_vif *avp = (void *)vif->drv_priv;
-	bool bs_valid = false;
-	int i;
 
 	ath_print(common, ATH_DBG_CONFIG, "Detach Interface\n");
 
@@ -1450,26 +1448,21 @@ static void ath9k_remove_interface(struct ieee80211_hw *hw,
 	if ((sc->sc_ah->opmode == NL80211_IFTYPE_AP) ||
 	    (sc->sc_ah->opmode == NL80211_IFTYPE_ADHOC) ||
 	    (sc->sc_ah->opmode == NL80211_IFTYPE_MESH_POINT)) {
+		/* Disable SWBA interrupt */
+		sc->sc_ah->imask &= ~ATH9K_INT_SWBA;
 		ath9k_ps_wakeup(sc);
+		ath9k_hw_set_interrupts(sc->sc_ah, sc->sc_ah->imask);
 		ath9k_hw_stoptxdma(sc->sc_ah, sc->beacon.beaconq);
 		ath9k_ps_restore(sc);
+		tasklet_kill(&sc->bcon_tasklet);
 	}
 
 	ath_beacon_return(sc, avp);
 	sc->sc_flags &= ~SC_OP_BEACONS;
 
-	for (i = 0; i < ARRAY_SIZE(sc->beacon.bslot); i++) {
-		if (sc->beacon.bslot[i] == vif) {
-			printk(KERN_DEBUG "%s: vif had allocated beacon "
-			       "slot\n", __func__);
-			sc->beacon.bslot[i] = NULL;
-			sc->beacon.bslot_aphy[i] = NULL;
-		} else if (sc->beacon.bslot[i])
-			bs_valid = true;
-	}
-	if (!bs_valid && (sc->sc_ah->imask & ATH9K_INT_SWBA)) {
-		/* Disable SWBA interrupt */
-		sc->sc_ah->imask &= ~ATH9K_INT_SWBA;
+	if (sc->nbcnvifs) {
+		/* Re-enable SWBA interrupt */
+		sc->sc_ah->imask |= ATH9K_INT_SWBA;
 		ath9k_ps_wakeup(sc);
 		ath9k_hw_set_interrupts(sc->sc_ah, sc->sc_ah->imask);
 		ath9k_ps_restore(sc);
-- 
1.7.3.2

Re: [PATCH] ath9k: fix beacon resource related race condition

[John W. Linville]

On Tue, Nov 30, 2010 at 04:36:37PM +0530, Rajkumar Manoharan wrote:
> While releasing beacon slot, it can be accessed by beacon tasklet.
> To avoid concurrency, beacon alert is disabled before
> freeing beacon resource and it will be enabled again in the
> persence of other beaconing mode interfaces.
> 
> Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>

Is this intended for 2.6.37?  It looks like it could apply there.

Could you be more specific about the effect of the bug you are fixing?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH] ath9k: fix beacon resource related race condition

[Rajkumar Manoharan]

On Fri, Dec 03, 2010 at 12:24:44AM +0530, John W. Linville wrote:
> On Tue, Nov 30, 2010 at 04:36:37PM +0530, Rajkumar Manoharan wrote:
> > While releasing beacon slot, it can be accessed by beacon tasklet.
> > To avoid concurrency, beacon alert is disabled before
> > freeing beacon resource and it will be enabled again in the
> > persence of other beaconing mode interfaces.
> > 
> > Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
> 
> Is this intended for 2.6.37?  It looks like it could apply there.

yes. I missed to mention. Thanks for pointing out.

> Could you be more specific about the effect of the bug you are fixing?

The beacon tasklet is accesssing the bslot info for beacon generation.
Meanwhile the same slot can be freed on interface deletion.
Actually the remove_interface disables the beacon alert after freeing the slot.
This may lead to null pointer access.

This patch disables SWBA and kills the beacon tasklet to prevent access
to the slot to be freed. After releasing the slot, swba will be enabled again
upon the availablity of beaconing interfaces.

--
Rajkumar