[PATCH] mac80211: Fix IBSS join

[PATCH] mac80211: Fix IBSS join

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Currently, when a station leaves an IBSS network, the
corresponding BSS is not dropped from cfg80211 if there are
other active stations in the network. But, the small
window that is present when trying to determine a station's
status based on IEEE80211_IBSS_MERGE_INTERVAL introduces
a race.

Instead of trying to keep the BSS, always remove it when
leaving an IBSS network. There is not much benefit to retain
the BSS entry since it will be added with a subsequent join
operation.

This fixes an issue where a dangling BSS entry causes ath9k
to wait for a beacon indefinitely.

Cc: <stable@vger.kernel.org>
Reported-by: Simon Wunderlich <sw@simonwunderlich.de>
Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
 net/mac80211/ibss.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 771080e..e1688cd 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -688,17 +688,18 @@ static int ieee80211_sta_active_ibss(struct ieee80211_sub_if_data *sdata)
 	return active;
 }
 
-static void ieee80211_ibss_disconnect(struct ieee80211_sub_if_data *sdata)
+static void ieee80211_ibss_disconnect(struct ieee80211_sub_if_data *sdata, bool leave)
 {
 	struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
 	struct ieee80211_local *local = sdata->local;
 	struct cfg80211_bss *cbss;
 	struct beacon_data *presp;
 	struct sta_info *sta;
-	int active_ibss;
+	int active_ibss = 0;
 	u16 capability;
 
-	active_ibss = ieee80211_sta_active_ibss(sdata);
+	if (!leave)
+		active_ibss = ieee80211_sta_active_ibss(sdata);
 
 	if (!active_ibss && !is_zero_ether_addr(ifibss->bssid)) {
 		capability = WLAN_CAPABILITY_IBSS;
@@ -765,7 +766,7 @@ static void ieee80211_csa_connection_drop_work(struct work_struct *work)
 
 	sdata_lock(sdata);
 
-	ieee80211_ibss_disconnect(sdata);
+	ieee80211_ibss_disconnect(sdata, false);
 	synchronize_rcu();
 	skb_queue_purge(&sdata->skb_queue);
 
@@ -1721,7 +1722,7 @@ int ieee80211_ibss_leave(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
 
-	ieee80211_ibss_disconnect(sdata);
+	ieee80211_ibss_disconnect(sdata, true);
 	ifibss->ssid_len = 0;
 	memset(ifibss->bssid, 0, ETH_ALEN);
 
-- 
1.8.5.3

Re: [PATCH] mac80211: Fix IBSS join

[Simon Wunderlich]

> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> 
> Currently, when a station leaves an IBSS network, the
> corresponding BSS is not dropped from cfg80211 if there are
> other active stations in the network. But, the small
> window that is present when trying to determine a station's
> status based on IEEE80211_IBSS_MERGE_INTERVAL introduces
> a race.
> 
> Instead of trying to keep the BSS, always remove it when
> leaving an IBSS network. There is not much benefit to retain
> the BSS entry since it will be added with a subsequent join
> operation.
> 
> This fixes an issue where a dangling BSS entry causes ath9k
> to wait for a beacon indefinitely.

Thank you Sujith, works fine with and without setting a fixed BSSID.

Feel free to add

Tested-by: Simon Wunderlich <sw@simonwunderlich.de>

Cheers,
    Simon

Re: [PATCH] mac80211: Fix IBSS join

[Johannes Berg]

On Tue, 2014-01-28 at 07:24 +0530, Sujith Manoharan wrote:


> -	ieee80211_ibss_disconnect(sdata);
> +	ieee80211_ibss_disconnect(sdata, false);

I'm not sure why you'd want to have 'false' here? If the network is
switching channel, then the old BSS entry is useless, no?

johannes

Re: [PATCH] mac80211: Fix IBSS join

[Simon Wunderlich]

> On Tue, 2014-01-28 at 07:24 +0530, Sujith Manoharan wrote:
> > -	ieee80211_ibss_disconnect(sdata);
> > +	ieee80211_ibss_disconnect(sdata, false);
> 
> I'm not sure why you'd want to have 'false' here? If the network is
> switching channel, then the old BSS entry is useless, no?

In the CSA case the BSS is still used, we modify the BSS entry in 
ieee80211_ibss_finish_csa()  (which is not really clean, but done by all the 
current CSA code).

Since the BSS is changed and the node continues to beacon, I think that's ok 
to do it like Sujith proposed.

Cheers,
    Simon

Re: [PATCH] mac80211: Fix IBSS join

[Sujith Manoharan]

Simon Wunderlich wrote:
> In the CSA case the BSS is still used, we modify the BSS entry in 
> ieee80211_ibss_finish_csa()  (which is not really clean, but done by all the 
> current CSA code).
> 
> Since the BSS is changed and the node continues to beacon, I think that's ok 
> to do it like Sujith proposed.

I think Johannes is right. If a station receives a CSA and is unable to switch
to the new channel, it leaves the IBSS network. In this case we don't need to
keep the BSS entry around, since we proceed to scan again.

Sujith

Re: [PATCH] mac80211: Fix IBSS join

[Johannes Berg]

On Thu, 2014-01-30 at 13:05 +0530, Sujith Manoharan wrote:
> Simon Wunderlich wrote:
> > In the CSA case the BSS is still used, we modify the BSS entry in 
> > ieee80211_ibss_finish_csa()  (which is not really clean, but done by all the 
> > current CSA code).
> > 
> > Since the BSS is changed and the node continues to beacon, I think that's ok 
> > to do it like Sujith proposed.
> 
> I think Johannes is right. If a station receives a CSA and is unable to switch
> to the new channel, it leaves the IBSS network. In this case we don't need to
> keep the BSS entry around, since we proceed to scan again.

Hah, I wasn't paying attention ... yeah I think you're right, it's the
*failure* case, in which we shouldn't care any more. Simon, you were
thinking of the successful case, but that doesn't call disconnect ...

johannes

Re: [PATCH] mac80211: Fix IBSS join

[Simon Wunderlich]

> On Thu, 2014-01-30 at 13:05 +0530, Sujith Manoharan wrote:
> > Simon Wunderlich wrote:
> > > In the CSA case the BSS is still used, we modify the BSS entry in
> > > ieee80211_ibss_finish_csa()  (which is not really clean, but done by
> > > all the current CSA code).
> > > 
> > > Since the BSS is changed and the node continues to beacon, I think
> > > that's ok to do it like Sujith proposed.
> > 
> > I think Johannes is right. If a station receives a CSA and is unable to
> > switch to the new channel, it leaves the IBSS network. In this case we
> > don't need to keep the BSS entry around, since we proceed to scan again.
> 
> Hah, I wasn't paying attention ... yeah I think you're right, it's the
> *failure* case, in which we shouldn't care any more. Simon, you were
> thinking of the successful case, but that doesn't call disconnect ...

Oh yes, I was just thinking, not reading. You are right, of course! Thanks for 
correcting me. :)

I see Sujith has already posted v2, thanks a lot!
     Simon