diff mbox series

Fix bss_is_ess() in case of an IBSS network

Message ID 20190912220429.32103-1-anzaki@gmail.com
State Changes Requested
Headers show
Series Fix bss_is_ess() in case of an IBSS network | expand

Commit Message

Ahmed Zaki Sept. 12, 2019, 10:04 p.m. UTC 
The check for the ESS or IBSS bits in the BSS caps is done via equality
to IEEE80211_CAP_ESS (0x01). This will only be true for AP/ESS and will
fail in case of IBSS (0x02).
---
 wpa_supplicant/events.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Jouni Malinen Sept. 13, 2019, 1:31 p.m. UTC | #1 
On Thu, Sep 12, 2019 at 04:04:29PM -0600, Ahmed Zaki wrote:
> The check for the ESS or IBSS bits in the BSS caps is done via equality
> to IEEE80211_CAP_ESS (0x01). This will only be true for AP/ESS and will
> fail in case of IBSS (0x02).

What do you mean with "fail" in this context? An IBSS is not an ESS, so
it sounds correct to me for bss_is_ess() to return 0 if the BSS is an
IBSS.

> diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
> @@ -906,8 +906,7 @@ static int bss_is_ess(struct wpa_bss *bss)
>  			IEEE80211_CAP_DMG_AP;
>  	}
>  
> -	return ((bss->caps & (IEEE80211_CAP_ESS | IEEE80211_CAP_IBSS)) ==
> -		IEEE80211_CAP_ESS);
> +	return (bss->caps & IEEE80211_CAP_ESS || bss->caps & IEEE80211_CAP_IBSS);

This would break bss_is_ess() by making it incorrectly claim an IBSS to
be an IBSS. Please note that the only caller of bss_is_ess() used to
originally do "if (bss->caps & IEEE80211_CAP_IBSS) continue" which is
what is still happening with the current bss_is_ess() implementation
while this proposed change would negate that.

What are you trying to fix with this?
Ahmed Zaki Sept. 13, 2019, 10:34 p.m. UTC | #2 
have an ath10k-ct IBSS network where the caps are advertised as
(0x0012 , IBSS Privacy). Upon joining the network, all units skip the
scan results and reject the network's BSS with the message ("   skip -
not ESS, PBSS, or MBSS") which is where bss_is_ess() is called. They
later create a new network and then merge after receiving a beacon
from another IBSS device.

I see now my original proposal is wrong, but can we add (ssid->mode !=
IEEE80211_MODE_IBSS) to the caller, like:

--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -1225,11 +1225,12 @@ struct wpa_ssid * wpa_scan_res_match(struct
wpa_supplicant *wpa_s,
                        continue;
                }

-               if (ssid->mode != WPAS_MODE_MESH && !bss_is_ess(bss) &&
-                   !bss_is_pbss(bss)) {
+               if (ssid->mode != WPAS_MODE_MESH &&
+                       ssid->mode != IEEE80211_MODE_IBSS &&
+                       !bss_is_ess(bss) && !bss_is_pbss(bss)) {
                        if (debug_print)
                                wpa_dbg(wpa_s, MSG_DEBUG,
-                                       "   skip - not ESS, PBSS, or MBSS");
+                                       "   skip - not ESS, IBSS,
PBSS, or MBSS");
                        continue;
                }

I tested that and it allows the units to join immediately after
getting the scan results. Isn't that what is intended?

Thanks.


On Fri, 13 Sep 2019 at 15:31, Jouni Malinen <j@w1.fi> wrote:
>
> On Thu, Sep 12, 2019 at 04:04:29PM -0600, Ahmed Zaki wrote:
> > The check for the ESS or IBSS bits in the BSS caps is done via equality
> > to IEEE80211_CAP_ESS (0x01). This will only be true for AP/ESS and will
> > fail in case of IBSS (0x02).
>
> What do you mean with "fail" in this context? An IBSS is not an ESS, so
> it sounds correct to me for bss_is_ess() to return 0 if the BSS is an
> IBSS.
>
> > diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
> > @@ -906,8 +906,7 @@ static int bss_is_ess(struct wpa_bss *bss)
> >                       IEEE80211_CAP_DMG_AP;
> >       }
> >
> > -     return ((bss->caps & (IEEE80211_CAP_ESS | IEEE80211_CAP_IBSS)) ==
> > -             IEEE80211_CAP_ESS);
> > +     return (bss->caps & IEEE80211_CAP_ESS || bss->caps & IEEE80211_CAP_IBSS);
>
> This would break bss_is_ess() by making it incorrectly claim an IBSS to
> be an IBSS. Please note that the only caller of bss_is_ess() used to
> originally do "if (bss->caps & IEEE80211_CAP_IBSS) continue" which is
> what is still happening with the current bss_is_ess() implementation
> while this proposed change would negate that.
>
> What are you trying to fix with this?
>
> --
> Jouni Malinen                                            PGP id EFC895FA
diff mbox series

Patch

diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index f28e4399e..c8208e53e 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -906,8 +906,7 @@  static int bss_is_ess(struct wpa_bss *bss)
 			IEEE80211_CAP_DMG_AP;
 	}
 
-	return ((bss->caps & (IEEE80211_CAP_ESS | IEEE80211_CAP_IBSS)) ==
-		IEEE80211_CAP_ESS);
+	return (bss->caps & IEEE80211_CAP_ESS || bss->caps & IEEE80211_CAP_IBSS);
 }