| Message ID | CAHc5FiVHh=wTEPjdYQnnCzXN4tLf1a0qJpikUMdm5sMgqQJM8w@mail.gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wed, 2012-09-26 at 16:52 +0200, Mykyta Iziumtsev wrote: > If auth/connect request to mac80211 fails with EALREADY - > there'll be retry after forced deauth/disconnect. The problem is that > wrong BSSID is used in this deauth/disconnect request. Instead of > BSSID of currently associated AP - BSSID of new AP will be used. > > For NL80211_CMD_DEAUTHENTICATE use correct cached BSSID value. > > For NL80211_CMD_DISCONNECT BSSID is not needed at all, > because mac80211 uses locally saved value. What kernel version are you testisng this against? The kernel behaviour changed significantly in this area, recently authentication isn't tracked in the kernel any more at all, only association. johannes
Hi Johannes, I'm using compat-wireless-3.2.5-1. I see that in nl80211_deauthenticate() NL80211_ATTR_MAC is mandatory. The same in compat-wireless-3.6-rc6-1. To clear possible misunderstanding: >> For NL80211_CMD_DEAUTHENTICATE use correct cached BSSID value. This caching is done in hostap.git: src/drivers/driver_nl80211.c , struct wpa_driver_nl80211_data->auth_bssid . /Mykyta On Wed, Sep 26, 2012 at 5:16 PM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Wed, 2012-09-26 at 16:52 +0200, Mykyta Iziumtsev wrote: >> If auth/connect request to mac80211 fails with EALREADY - >> there'll be retry after forced deauth/disconnect. The problem is that >> wrong BSSID is used in this deauth/disconnect request. Instead of >> BSSID of currently associated AP - BSSID of new AP will be used. >> >> For NL80211_CMD_DEAUTHENTICATE use correct cached BSSID value. >> >> For NL80211_CMD_DISCONNECT BSSID is not needed at all, >> because mac80211 uses locally saved value. > > What kernel version are you testisng this against? The kernel behaviour > changed significantly in this area, recently authentication isn't > tracked in the kernel any more at all, only association. > > johannes >
Hi, > I'm using compat-wireless-3.2.5-1. I see that in nl80211_deauthenticate() > NL80211_ATTR_MAC is mandatory. The same in compat-wireless-3.6-rc6-1. Yes, but 3.2 and 3.5 behave quite differently in this area. In 3.5, you don't actually need to deauthenticate all the time, the whole "state mismatch" handling isn't necessary. Therefore, I was curious if you tested this before those changes, but then you did. > To clear possible misunderstanding: > >> For NL80211_CMD_DEAUTHENTICATE use correct cached BSSID value. > This caching is done in hostap.git: src/drivers/driver_nl80211.c , > struct wpa_driver_nl80211_data->auth_bssid . Right, I understand. johannes
On Wed, 2012-09-26 at 16:52 +0200, Mykyta Iziumtsev wrote: > @@ -4648,7 +4650,7 @@ retry: > "nl80211: MLME command failed (auth): ret=%d (%s)", > ret, strerror(-ret)); > count++; > - if (ret == -EALREADY && count == 1 && params->bssid && > + if (ret == -EALREADY && count == 1 && > !params->local_state_change) { > /* > * mac80211 does not currently accept new > @@ -4658,7 +4660,7 @@ retry: > wpa_printf(MSG_DEBUG, "nl80211: Retry authentication " > "after forced deauthentication"); > wpa_driver_nl80211_deauthenticate( > - bss, params->bssid, > + bss, prev_auth_bssid, > WLAN_REASON_PREV_AUTH_NOT_VALID); > nlmsg_free(msg); > goto retry; I'm not convinced this is correct. If cfg80211 returns -EALREADY, it means "already authenticated with this BSS". So it seems using params->bssid here would be correct in order to re-authenticate. The case where we can't authenticate because it's already authenticated with more APs than it would like to track will return -ENOSPC. Note like I said, starting from kernel 3.4 all this went away. > @@ -6617,51 +6619,11 @@ nla_put_failure: > } > > > -static unsigned int nl80211_get_assoc_bssid(struct > wpa_driver_nl80211_data *drv, > - u8 *bssid) > +static int nl80211_disconnect(struct wpa_driver_nl80211_data *drv) > { > - struct nl_msg *msg; > - int ret; > - struct nl80211_bss_info_arg arg; > - > - os_memset(&arg, 0, sizeof(arg)); > - msg = nlmsg_alloc(); > - if (!msg) > - goto nla_put_failure; > - > - nl80211_cmd(drv, msg, NLM_F_DUMP, NL80211_CMD_GET_SCAN); > - NLA_PUT_U32(msg, NL80211_ATTR_IFINDEX, drv->ifindex); > - > - arg.drv = drv; > - ret = send_and_recv_msgs(drv, msg, bss_info_handler, &arg); > - msg = NULL; > - if (ret == 0) { > - if (is_zero_ether_addr(arg.assoc_bssid)) > - return -ENOTCONN; > - os_memcpy(bssid, arg.assoc_bssid, ETH_ALEN); > - return 0; > - } > - wpa_printf(MSG_DEBUG, "nl80211: Scan result fetch failed: ret=%d " > - "(%s)", ret, strerror(-ret)); > -nla_put_failure: > - nlmsg_free(msg); > - return drv->assoc_freq; > -} > - > - > -static int nl80211_disconnect(struct wpa_driver_nl80211_data *drv, > - const u8 *bssid) > -{ > - u8 addr[ETH_ALEN]; > - > - if (bssid == NULL) { > - int res = nl80211_get_assoc_bssid(drv, addr); > - if (res) > - return res; > - bssid = addr; > - } > - > - return wpa_driver_nl80211_disconnect(drv, bssid, > + u8 zero_addr[ETH_ALEN]; > + os_memset(zero_addr, 0, ETH_ALEN); > + return wpa_driver_nl80211_disconnect(drv, zero_addr, > WLAN_REASON_PREV_AUTH_NOT_VALID); > } Maybe this part should be a separate patch since it's a simplification due to the fact that DISCONNECT doesn't care about the BSSID? johannes
On Fri, Sep 28, 2012 at 9:32 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Wed, 2012-09-26 at 16:52 +0200, Mykyta Iziumtsev wrote: > >> @@ -4648,7 +4650,7 @@ retry: >> "nl80211: MLME command failed (auth): ret=%d (%s)", >> ret, strerror(-ret)); >> count++; >> - if (ret == -EALREADY && count == 1 && params->bssid && >> + if (ret == -EALREADY && count == 1 && >> !params->local_state_change) { >> /* >> * mac80211 does not currently accept new >> @@ -4658,7 +4660,7 @@ retry: >> wpa_printf(MSG_DEBUG, "nl80211: Retry authentication " >> "after forced deauthentication"); >> wpa_driver_nl80211_deauthenticate( >> - bss, params->bssid, >> + bss, prev_auth_bssid, >> WLAN_REASON_PREV_AUTH_NOT_VALID); >> nlmsg_free(msg); >> goto retry; > > I'm not convinced this is correct. If cfg80211 returns -EALREADY, it > means "already authenticated with this BSS". So it seems using > params->bssid here would be correct in order to re-authenticate. > > The case where we can't authenticate because it's already authenticated > with more APs than it would like to track will return -ENOSPC. Note like > I said, starting from kernel 3.4 all this went away. This code is actually workaround for older versions of mac80211 returning -EALREADY when we're already connected to some BSS. Please refer to commit 6d6f4bb87f33278aed133875d0d561eb55d7ae59 in hostap.git . So, we should expect -EALREADY not only in "already authenticated with this BSS" case, but in "already authenticated with *any* BSS" case. Of course, mac80211 doesn't have this problem any more when userspace SME is in use, but it still returns -EALREADY when "connect" API is used (in compat-wireless-3.2.5-1). >> @@ -6617,51 +6619,11 @@ nla_put_failure: >> } >> >> >> -static unsigned int nl80211_get_assoc_bssid(struct >> wpa_driver_nl80211_data *drv, >> - u8 *bssid) >> +static int nl80211_disconnect(struct wpa_driver_nl80211_data *drv) >> { >> - struct nl_msg *msg; >> - int ret; >> - struct nl80211_bss_info_arg arg; >> - >> - os_memset(&arg, 0, sizeof(arg)); >> - msg = nlmsg_alloc(); >> - if (!msg) >> - goto nla_put_failure; >> - >> - nl80211_cmd(drv, msg, NLM_F_DUMP, NL80211_CMD_GET_SCAN); >> - NLA_PUT_U32(msg, NL80211_ATTR_IFINDEX, drv->ifindex); >> - >> - arg.drv = drv; >> - ret = send_and_recv_msgs(drv, msg, bss_info_handler, &arg); >> - msg = NULL; >> - if (ret == 0) { >> - if (is_zero_ether_addr(arg.assoc_bssid)) >> - return -ENOTCONN; >> - os_memcpy(bssid, arg.assoc_bssid, ETH_ALEN); >> - return 0; >> - } >> - wpa_printf(MSG_DEBUG, "nl80211: Scan result fetch failed: ret=%d " >> - "(%s)", ret, strerror(-ret)); >> -nla_put_failure: >> - nlmsg_free(msg); >> - return drv->assoc_freq; >> -} >> - >> - >> -static int nl80211_disconnect(struct wpa_driver_nl80211_data *drv, >> - const u8 *bssid) >> -{ >> - u8 addr[ETH_ALEN]; >> - >> - if (bssid == NULL) { >> - int res = nl80211_get_assoc_bssid(drv, addr); >> - if (res) >> - return res; >> - bssid = addr; >> - } >> - >> - return wpa_driver_nl80211_disconnect(drv, bssid, >> + u8 zero_addr[ETH_ALEN]; >> + os_memset(zero_addr, 0, ETH_ALEN); >> + return wpa_driver_nl80211_disconnect(drv, zero_addr, >> WLAN_REASON_PREV_AUTH_NOT_VALID); >> } > > Maybe this part should be a separate patch since it's a simplification > due to the fact that DISCONNECT doesn't care about the BSSID? Right, I'll split the patch into two. > johannes >
On Fri, 2012-09-28 at 09:52 +0200, Mykyta Iziumtsev wrote: > >> @@ -4658,7 +4660,7 @@ retry: > >> wpa_printf(MSG_DEBUG, "nl80211: Retry authentication " > >> "after forced deauthentication"); > >> wpa_driver_nl80211_deauthenticate( > >> - bss, params->bssid, > >> + bss, prev_auth_bssid, > >> WLAN_REASON_PREV_AUTH_NOT_VALID); > >> nlmsg_free(msg); > >> goto retry; > > > > I'm not convinced this is correct. If cfg80211 returns -EALREADY, it > > means "already authenticated with this BSS". So it seems using > > params->bssid here would be correct in order to re-authenticate. > > > > The case where we can't authenticate because it's already authenticated > > with more APs than it would like to track will return -ENOSPC. Note like > > I said, starting from kernel 3.4 all this went away. > > This code is actually workaround for older versions of mac80211 returning > -EALREADY when we're already connected to some BSS. Please refer to > commit 6d6f4bb87f33278aed133875d0d561eb55d7ae59 in hostap.git . Yes, I know. > So, we should expect -EALREADY not only in "already authenticated with this BSS" > case, but in "already authenticated with *any* BSS" case. No, that's not true. > Of course, mac80211 doesn't have this problem any more when userspace > SME is in use, but it still returns -EALREADY when "connect" API is used > (in compat-wireless-3.2.5-1). Well you're not modifying the CONNECT API use though, you're modifying the AUTHENTICATE API in a way that actually breaks the -EALREADY workaround. I could see a bit of value in doing the clear_state_mismatch() workaround if you get -EALREADY or -ENOSPC, but what you're doing here seems to break it. johannes
Hi Johannes, Maybe I'm missing something obvious. I'll try to analyse code deeper, and get back with proof links. Thank you! /Mykyta On Fri, Sep 28, 2012 at 12:36 PM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Fri, 2012-09-28 at 09:52 +0200, Mykyta Iziumtsev wrote: > >> >> @@ -4658,7 +4660,7 @@ retry: >> >> wpa_printf(MSG_DEBUG, "nl80211: Retry authentication " >> >> "after forced deauthentication"); >> >> wpa_driver_nl80211_deauthenticate( >> >> - bss, params->bssid, >> >> + bss, prev_auth_bssid, >> >> WLAN_REASON_PREV_AUTH_NOT_VALID); >> >> nlmsg_free(msg); >> >> goto retry; >> > >> > I'm not convinced this is correct. If cfg80211 returns -EALREADY, it >> > means "already authenticated with this BSS". So it seems using >> > params->bssid here would be correct in order to re-authenticate. >> > >> > The case where we can't authenticate because it's already authenticated >> > with more APs than it would like to track will return -ENOSPC. Note like >> > I said, starting from kernel 3.4 all this went away. >> >> This code is actually workaround for older versions of mac80211 returning >> -EALREADY when we're already connected to some BSS. Please refer to >> commit 6d6f4bb87f33278aed133875d0d561eb55d7ae59 in hostap.git . > > Yes, I know. > >> So, we should expect -EALREADY not only in "already authenticated with this BSS" >> case, but in "already authenticated with *any* BSS" case. > > No, that's not true. > >> Of course, mac80211 doesn't have this problem any more when userspace >> SME is in use, but it still returns -EALREADY when "connect" API is used >> (in compat-wireless-3.2.5-1). > > Well you're not modifying the CONNECT API use though, you're modifying > the AUTHENTICATE API in a way that actually breaks the -EALREADY > workaround. > > I could see a bit of value in doing the clear_state_mismatch() > workaround if you get -EALREADY or -ENOSPC, but what you're doing here > seems to break it. > > johannes >
On Sun, 2012-09-30 at 21:50 +0200, Mykyta Iziumtsev wrote: > Hi Johannes, > > Maybe I'm missing something obvious. > I'll try to analyse code deeper, and get back with proof links. Alright, sure, I'm always happy to be proven wrong :-) johannes
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c index a5659c9..0a9f836 100644 --- a/src/drivers/driver_nl80211.c +++ b/src/drivers/driver_nl80211.c @@ -4565,11 +4565,13 @@ static int wpa_driver_nl80211_authenticate( enum nl80211_iftype nlmode; int count = 0; int is_retry; + u8 prev_auth_bssid[ETH_ALEN]; is_retry = drv->retry_auth; drv->retry_auth = 0; drv->associated = 0; + os_memcpy(prev_auth_bssid, drv->auth_bssid, ETH_ALEN); os_memset(drv->auth_bssid, 0, ETH_ALEN); /* FIX: IBSS mode */ nlmode = params->p2p ? @@ -4648,7 +4650,7 @@ retry: "nl80211: MLME command failed (auth): ret=%d (%s)", ret, strerror(-ret)); count++; - if (ret == -EALREADY && count == 1 && params->bssid && + if (ret == -EALREADY && count == 1 && !params->local_state_change) { /* * mac80211 does not currently accept new @@ -4658,7 +4660,7 @@ retry: wpa_printf(MSG_DEBUG, "nl80211: Retry authentication " "after forced deauthentication"); wpa_driver_nl80211_deauthenticate( - bss, params->bssid, + bss, prev_auth_bssid, WLAN_REASON_PREV_AUTH_NOT_VALID); nlmsg_free(msg); goto retry; @@ -6617,51 +6619,11 @@ nla_put_failure: } -static unsigned int nl80211_get_assoc_bssid(struct wpa_driver_nl80211_data *drv, - u8 *bssid) +static int nl80211_disconnect(struct wpa_driver_nl80211_data *drv) { - struct nl_msg *msg; - int ret; - struct nl80211_bss_info_arg arg; - - os_memset(&arg, 0, sizeof(arg)); - msg = nlmsg_alloc(); - if (!msg) - goto nla_put_failure; - - nl80211_cmd(drv, msg, NLM_F_DUMP, NL80211_CMD_GET_SCAN); - NLA_PUT_U32(msg, NL80211_ATTR_IFINDEX, drv->ifindex); - - arg.drv = drv; - ret = send_and_recv_msgs(drv, msg, bss_info_handler, &arg); - msg = NULL; - if (ret == 0) { - if (is_zero_ether_addr(arg.assoc_bssid)) - return -ENOTCONN; - os_memcpy(bssid, arg.assoc_bssid, ETH_ALEN); - return 0; - } - wpa_printf(MSG_DEBUG, "nl80211: Scan result fetch failed: ret=%d " - "(%s)", ret, strerror(-ret)); -nla_put_failure: - nlmsg_free(msg); - return drv->assoc_freq; -} - - -static int nl80211_disconnect(struct wpa_driver_nl80211_data *drv, - const u8 *bssid) -{ - u8 addr[ETH_ALEN]; - - if (bssid == NULL) { - int res = nl80211_get_assoc_bssid(drv, addr); - if (res) - return res; - bssid = addr; - } - - return wpa_driver_nl80211_disconnect(drv, bssid, + u8 zero_addr[ETH_ALEN]; + os_memset(zero_addr, 0, ETH_ALEN); + return wpa_driver_nl80211_disconnect(drv, zero_addr, WLAN_REASON_PREV_AUTH_NOT_VALID); }