Search Linux Wireless

Re: [PATCH] cfg80211: Fix race in bss timeout

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2011-12-07 at 21:00 +0530, Vasanthakumar Thiagarajan wrote:

> > >  	nl80211_send_roamed(wiphy_to_dev(wdev->wiphy), wdev->netdev, bssid,
> > >  			    req_ie, req_ie_len, resp_ie, resp_ie_len,
> > >  			    GFP_KERNEL);
> > > @@ -615,40 +612,65 @@ void __cfg80211_roamed(struct wireless_dev *wdev,
> > >  	wdev->wext.prev_bssid_valid = true;
> > >  	wireless_send_event(wdev->netdev, SIOCGIWAP, &wrqu, NULL);
> > >  #endif
> > > +
> > > +	return;
> > > +out:
> > > +	if (bss)
> > > +		cfg80211_put_bss(bss);
> > >  }
> > 
> > Doesn't that leak the reference if you return? It'll also give you an
> > smatch warning since the function assumes the "bss" pointer that was
> > passed in is not NULL, no?
> 
> Oops, sorry, i'll fix it. I may need to run smatch as well.

No need to run smatch -- but the function does assume that the bss
pointer passed in is not NULL, so it should never need to check if it's
NULL (never mind the fact that you can pass NULL to put_bss too)

> > > +static void cfg80211_roamed_bss(struct net_device *dev,
> > > +				struct cfg80211_bss *bss, const u8 *req_ie,
> > > +				size_t req_ie_len, const u8 *resp_ie,
> > > +				size_t resp_ie_len, gfp_t gfp)
> > >  {
> > >  	struct wireless_dev *wdev = dev->ieee80211_ptr;
> > >  	struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
> > >  	struct cfg80211_event *ev;
> > >  	unsigned long flags;
> > >  
> > > -	CFG80211_DEV_WARN_ON(wdev->sme_state != CFG80211_SME_CONNECTED);
> > >  
> > >  	ev = kzalloc(sizeof(*ev) + req_ie_len + resp_ie_len, gfp);
> > 
> > Why remove that warning? Also maybe do something like
> > 
> > 	if (WARN_ON(!bss))
> > 		return;
> > 
> > (before allocating memory)
> 
> These warnings are added in cfg80211_roamed().

But this can be called directly by the driver with a NULL BSS.

Ohh. I see what you did, you didn't allow drivers calling this now. I
think you should export this function still since otherwise the race
windows might get tiny, but isn't actually completely closed (the first
get_bss() might find it, the next a millisecond later not)

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux