* [PATCH] Fix NetworkManager/wpa_supplicant race condition @ 2007-04-18 16:40 Tim Gardner 2007-04-18 17:22 ` Dan Williams 0 siblings, 1 reply; 5+ messages in thread From: Tim Gardner @ 2007-04-18 16:40 UTC (permalink / raw) To: linux-wireless; +Cc: kernel-team, Ben Collins, mjg59 [-- Attachment #1: Type: text/plain, Size: 710 bytes --] This patch fixes an assumption made by wpa_supplicant. Any time wpa_supplicant requests to set an ESSID (e.g., associate), it expects an event notifying that association has completed. If the Networkmanager has already setup an association, such as for an open auth AP, then the request to associate by wpa_supplicant will be ignored. If Networkmanager is requested to restart the connection, such as by clicking on the SSID, then wpa_supplicant is allowed to build the association from scratch, which always works. https://bugs.launchpad.net/ubuntu/+source/linux-source-2.6.20/+bug/103768 By always emitting this event, am I causing any unintended side effects? rtg -- Tim Gardner tim.gardner@ubuntu.com [-- Attachment #2: bug_103768 --] [-- Type: text/plain, Size: 888 bytes --] diff --git a/net/ieee80211/softmac/ieee80211softmac_wx.c b/net/ieee80211/softmac/ieee80211softmac_wx.c index fa2f7da..cc2e8ba 100644 --- a/net/ieee80211/softmac/ieee80211softmac_wx.c +++ b/net/ieee80211/softmac/ieee80211softmac_wx.c @@ -88,6 +88,13 @@ ieee80211softmac_wx_set_essid(struct net_device *net_dev, !memcmp(n->essid.data, extra, n->essid.len)) { dprintk(KERN_INFO PFX "Already associating or associated to "MAC_FMT"\n", MAC_ARG(sm->associnfo.bssid)); + /* wpa_supplicant expects an association event, regardless of prior + * association state. If associating, then the associnfo.work task + * will send the appropriate event. + */ + if (sm->associnfo.associated) + ieee80211softmac_call_events_locked(sm, + IEEE80211SOFTMAC_EVENT_ASSOCIATED, n); goto out; } else { dprintk(KERN_INFO PFX "Canceling existing associate request!\n"); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix NetworkManager/wpa_supplicant race condition 2007-04-18 16:40 [PATCH] Fix NetworkManager/wpa_supplicant race condition Tim Gardner @ 2007-04-18 17:22 ` Dan Williams 2007-04-18 19:18 ` Tim Gardner 0 siblings, 1 reply; 5+ messages in thread From: Dan Williams @ 2007-04-18 17:22 UTC (permalink / raw) To: Tim Gardner; +Cc: linux-wireless, kernel-team, Ben Collins, mjg59 On Wed, 2007-04-18 at 10:40 -0600, Tim Gardner wrote: > This patch fixes an assumption made by wpa_supplicant. Any time > wpa_supplicant requests to set an ESSID (e.g., associate), it expects an > event notifying that association has completed. If the Networkmanager > has already setup an association, such as for an open auth AP, then the > request to associate by wpa_supplicant will be ignored. > > If Networkmanager is requested to restart the connection, such as by > clicking on the SSID, then wpa_supplicant is allowed to build the > association from scratch, which always works. > > https://bugs.launchpad.net/ubuntu/+source/linux-source-2.6.20/+bug/103768 > > By always emitting this event, am I causing any unintended side effects? Interesting. Setting the SSID should _always_ trigger a reassociation, and therefore eventually trigger an SIOCGIWAP event to userspace. So this patch looks right from the behavioral point of view. But it doesn't look right from the technical perspective. Why isn't softmac trying to reassociate? Does it automatically reassociate if parameters, like auth mode, keys, WPA, etc are set and so therefore, when it comes around to the SSID being set it doesn't really matter? If so, we should still be sending the event, and this patch is OK. But can somebody _guarantee_ that if I authenticate to an AP, then later call SIOCSIWENCODE, and then SIOCSIWESSID to the same SSID, that the new WEP keys have been applied and a reassociation has occurred? If the auth mode is shared key, a reassociation attempt needs to happen. WEXT convention is that setting the SSID or BSSID triggers reassociation with the current parameters. I'd argue that softmac should be starting the association process over again when either SIOCSIWESSID or SIOCSIWBSSID is called. Dan > rtg > plain text document attachment (bug_103768) > diff --git a/net/ieee80211/softmac/ieee80211softmac_wx.c b/net/ieee80211/softmac/ieee80211softmac_wx.c > index fa2f7da..cc2e8ba 100644 > --- a/net/ieee80211/softmac/ieee80211softmac_wx.c > +++ b/net/ieee80211/softmac/ieee80211softmac_wx.c > @@ -88,6 +88,13 @@ ieee80211softmac_wx_set_essid(struct net_device *net_dev, > !memcmp(n->essid.data, extra, n->essid.len)) { > dprintk(KERN_INFO PFX "Already associating or associated to "MAC_FMT"\n", > MAC_ARG(sm->associnfo.bssid)); > + /* wpa_supplicant expects an association event, regardless of prior > + * association state. If associating, then the associnfo.work task > + * will send the appropriate event. > + */ > + if (sm->associnfo.associated) > + ieee80211softmac_call_events_locked(sm, > + IEEE80211SOFTMAC_EVENT_ASSOCIATED, n); > goto out; > } else { > dprintk(KERN_INFO PFX "Canceling existing associate request!\n"); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix NetworkManager/wpa_supplicant race condition 2007-04-18 17:22 ` Dan Williams @ 2007-04-18 19:18 ` Tim Gardner 2007-04-19 3:04 ` Dan Williams 0 siblings, 1 reply; 5+ messages in thread From: Tim Gardner @ 2007-04-18 19:18 UTC (permalink / raw) To: Dan Williams; +Cc: linux-wireless, kernel-team Dan Williams wrote: > On Wed, 2007-04-18 at 10:40 -0600, Tim Gardner wrote: >> This patch fixes an assumption made by wpa_supplicant. Any time >> wpa_supplicant requests to set an ESSID (e.g., associate), it expects an >> event notifying that association has completed. If the Networkmanager >> has already setup an association, such as for an open auth AP, then the >> request to associate by wpa_supplicant will be ignored. >> >> If Networkmanager is requested to restart the connection, such as by >> clicking on the SSID, then wpa_supplicant is allowed to build the >> association from scratch, which always works. >> >> https://bugs.launchpad.net/ubuntu/+source/linux-source-2.6.20/+bug/103768 >> >> By always emitting this event, am I causing any unintended side effects? > > Interesting. Setting the SSID should _always_ trigger a reassociation, > and therefore eventually trigger an SIOCGIWAP event to userspace. So > this patch looks right from the behavioral point of view. > > But it doesn't look right from the technical perspective. Why isn't > softmac trying to reassociate? Does it automatically reassociate if > parameters, like auth mode, keys, WPA, etc are set and so therefore, > when it comes around to the SSID being set it doesn't really matter? If > so, we should still be sending the event, and this patch is OK. But can > somebody _guarantee_ that if I authenticate to an AP, then later call > SIOCSIWENCODE, and then SIOCSIWESSID to the same SSID, that the new WEP > keys have been applied and a reassociation has occurred? If the auth > mode is shared key, a reassociation attempt needs to happen. > > WEXT convention is that setting the SSID or BSSID triggers reassociation > with the current parameters. I'd argue that softmac should be starting > the association process over again when either SIOCSIWESSID or > SIOCSIWBSSID is called. > > Dan > >> rtg >> plain text document attachment (bug_103768) >> diff --git a/net/ieee80211/softmac/ieee80211softmac_wx.c b/net/ieee80211/softmac/ieee80211softmac_wx.c >> index fa2f7da..cc2e8ba 100644 >> --- a/net/ieee80211/softmac/ieee80211softmac_wx.c >> +++ b/net/ieee80211/softmac/ieee80211softmac_wx.c >> @@ -88,6 +88,13 @@ ieee80211softmac_wx_set_essid(struct net_device *net_dev, >> !memcmp(n->essid.data, extra, n->essid.len)) { >> dprintk(KERN_INFO PFX "Already associating or associated to "MAC_FMT"\n", >> MAC_ARG(sm->associnfo.bssid)); >> + /* wpa_supplicant expects an association event, regardless of prior >> + * association state. If associating, then the associnfo.work task >> + * will send the appropriate event. >> + */ >> + if (sm->associnfo.associated) >> + ieee80211softmac_call_events_locked(sm, >> + IEEE80211SOFTMAC_EVENT_ASSOCIATED, n); >> goto out; >> } else { >> dprintk(KERN_INFO PFX "Canceling existing associate request!\n"); > I don't think simply reassociating is sufficient. Consider what happens if the authentication algorithm is changed. ieee80211_wx_set_auth() does nothing more then set some values. A subsequent attempt to reassociate would get all hosed up. A quick look at ieee80211_ioctl_siwessid() shows that it always forces an authentication cycle. I'll experiment a little to see how the Softmac can be driven through an authentication/association cycle on calls to SIOCSIWESSID or SIOCSIWAP. rtg -- Tim Gardner tim.gardner@ubuntu.com ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix NetworkManager/wpa_supplicant race condition 2007-04-18 19:18 ` Tim Gardner @ 2007-04-19 3:04 ` Dan Williams 2007-04-19 15:51 ` Tim Gardner 0 siblings, 1 reply; 5+ messages in thread From: Dan Williams @ 2007-04-19 3:04 UTC (permalink / raw) To: Tim Gardner; +Cc: linux-wireless, kernel-team On Wed, 2007-04-18 at 13:18 -0600, Tim Gardner wrote: > Dan Williams wrote: > > On Wed, 2007-04-18 at 10:40 -0600, Tim Gardner wrote: > >> This patch fixes an assumption made by wpa_supplicant. Any time > >> wpa_supplicant requests to set an ESSID (e.g., associate), it expects an > >> event notifying that association has completed. If the Networkmanager > >> has already setup an association, such as for an open auth AP, then the > >> request to associate by wpa_supplicant will be ignored. > >> > >> If Networkmanager is requested to restart the connection, such as by > >> clicking on the SSID, then wpa_supplicant is allowed to build the > >> association from scratch, which always works. > >> > >> https://bugs.launchpad.net/ubuntu/+source/linux-source-2.6.20/+bug/103768 > >> > >> By always emitting this event, am I causing any unintended side effects? > > > > Interesting. Setting the SSID should _always_ trigger a reassociation, > > and therefore eventually trigger an SIOCGIWAP event to userspace. So > > this patch looks right from the behavioral point of view. > > > > But it doesn't look right from the technical perspective. Why isn't > > softmac trying to reassociate? Does it automatically reassociate if > > parameters, like auth mode, keys, WPA, etc are set and so therefore, > > when it comes around to the SSID being set it doesn't really matter? If > > so, we should still be sending the event, and this patch is OK. But can > > somebody _guarantee_ that if I authenticate to an AP, then later call > > SIOCSIWENCODE, and then SIOCSIWESSID to the same SSID, that the new WEP > > keys have been applied and a reassociation has occurred? If the auth > > mode is shared key, a reassociation attempt needs to happen. > > > > WEXT convention is that setting the SSID or BSSID triggers reassociation > > with the current parameters. I'd argue that softmac should be starting > > the association process over again when either SIOCSIWESSID or > > SIOCSIWBSSID is called. > > > > Dan > > > >> rtg > >> plain text document attachment (bug_103768) > >> diff --git a/net/ieee80211/softmac/ieee80211softmac_wx.c b/net/ieee80211/softmac/ieee80211softmac_wx.c > >> index fa2f7da..cc2e8ba 100644 > >> --- a/net/ieee80211/softmac/ieee80211softmac_wx.c > >> +++ b/net/ieee80211/softmac/ieee80211softmac_wx.c > >> @@ -88,6 +88,13 @@ ieee80211softmac_wx_set_essid(struct net_device *net_dev, > >> !memcmp(n->essid.data, extra, n->essid.len)) { > >> dprintk(KERN_INFO PFX "Already associating or associated to "MAC_FMT"\n", > >> MAC_ARG(sm->associnfo.bssid)); > >> + /* wpa_supplicant expects an association event, regardless of prior > >> + * association state. If associating, then the associnfo.work task > >> + * will send the appropriate event. > >> + */ > >> + if (sm->associnfo.associated) > >> + ieee80211softmac_call_events_locked(sm, > >> + IEEE80211SOFTMAC_EVENT_ASSOCIATED, n); > >> goto out; > >> } else { > >> dprintk(KERN_INFO PFX "Canceling existing associate request!\n"); > > > > I don't think simply reassociating is sufficient. Consider what happens > if the authentication algorithm is changed. ieee80211_wx_set_auth() does > nothing more then set some values. A subsequent attempt to reassociate > would get all hosed up. When I say "reassociate", I mean a new association cycle to the _same_ BSSID/SSID using the settings that are current at that point in time. I guess I should have made that clear. Dan > A quick look at ieee80211_ioctl_siwessid() shows that it always forces > an authentication cycle. > > I'll experiment a little to see how the Softmac can be driven through an > authentication/association cycle on calls to SIOCSIWESSID or SIOCSIWAP. > > rtg ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix NetworkManager/wpa_supplicant race condition 2007-04-19 3:04 ` Dan Williams @ 2007-04-19 15:51 ` Tim Gardner 0 siblings, 0 replies; 5+ messages in thread From: Tim Gardner @ 2007-04-19 15:51 UTC (permalink / raw) To: Dan Williams; +Cc: Tim Gardner, linux-wireless Dan Williams wrote: snip, snip > > When I say "reassociate", I mean a new association cycle to the _same_ > BSSID/SSID using the settings that are current at that point in time. I > guess I should have made that clear. > > Dan > >> A quick look at ieee80211_ioctl_siwessid() shows that it always forces >> an authentication cycle. >> >> I'll experiment a little to see how the Softmac can be driven through an >> authentication/association cycle on calls to SIOCSIWESSID or SIOCSIWAP. >> >> rtg Dan, I agree that driving the SoftMAC protocol through an auth/assoc cycle when changing ESSID or BSSID is architecturally the right thing to do. However, the required changes are a bit too extensive for me to add to a recently released kernel (Feisty). I'll have to get back to this when I have more freedom to whack and slash. In the meantime, I still think emitting an ASSOC event is a good interim fix. I'll be testing it over the next few days within the Ubuntu community. rtg -- Tim Gardner tim.gardner@ubuntu.com ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-04-19 15:51 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-04-18 16:40 [PATCH] Fix NetworkManager/wpa_supplicant race condition Tim Gardner 2007-04-18 17:22 ` Dan Williams 2007-04-18 19:18 ` Tim Gardner 2007-04-19 3:04 ` Dan Williams 2007-04-19 15:51 ` Tim Gardner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).