* Re: [PATCH] brcmfmac: Turn off ARP offloading when configured for AP.
From: Greg KH @ 2013-06-19 14:19 UTC (permalink / raw)
To: Arend van Spriel; +Cc: stable, linux-wireless, Hante Meuleman, John W. Linville
In-Reply-To: <51C15D4D.6040009@broadcom.com>
On Wed, Jun 19, 2013 at 09:27:09AM +0200, Arend van Spriel wrote:
> On 06/06/2013 10:55 AM, Arend van Spriel wrote:
> >From: Hante Meuleman <meuleman@broadcom.com>
>
> Hi Greg,
>
> I noticed your review announcement for v3.9.7 and did not see the
> change below. I sent it to stable because the original upstream
> commit did not apply. Did I miss some step in the process?
Ah, somehow I missed your patch, sorry about that, I do have it in my
mbox. What kernel tree(s) do you want it to be applied to? I'll pick
it up in my next round of releases.
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC 2/2] mac80211: Fix bss ref leak.
From: Johannes Berg @ 2013-06-19 14:13 UTC (permalink / raw)
To: Ben Greear; +Cc: linux-wireless
In-Reply-To: <51C08BF3.7050706@candelatech.com>
On Tue, 2013-06-18 at 09:33 -0700, Ben Greear wrote:
> On 06/18/2013 08:26 AM, Johannes Berg wrote:
> > On Mon, 2013-06-17 at 14:32 -0700, greearb@candelatech.com wrote:
> >
> >> static void ieee80211_destroy_assoc_data(struct ieee80211_sub_if_data *sdata,
> >> - bool assoc)
> >> + bool assoc, bool put_bss)
> >
> > Do we _really_ need another argument? Shouldn't it always be put in the
> > non-assoc case anyway, at least if non-NULL?
>
> I don't think so. Check out the ieee80211_rx_mgmt_assoc_resp method.
>
> if (status_code != WLAN_STATUS_SUCCESS) {
> sdata_info(sdata, "%pM denied association (code=%d)\n",
> mgmt->sa, status_code);
> ieee80211_destroy_assoc_data(sdata, false, false);
>
> This passes in false as 'assoc', but we should not free bss here because
> it is being passed back to the calling method, and the return
> code of RX_MGMT_CFG80211_RX_ASSOC means bss should eventually
> be consumed by the cfg80211 logic.
>
> Of course, this is all 'as far as I can tell'.
>
> I sort of like the second boolean because it forces the caller to
> think about whether bss should be freed or not...
I just posted a patch to hand it back and never free it in mac80211 at
all, that also allows cfg80211_hold_bss() it across the assoc process
which fixes a (possibly more theoretical) expiry issue.
johannes
^ permalink raw reply
* Re: [ath9k-devel] ath9k_htc: station unable to authenticate
From: Johannes Berg @ 2013-06-19 14:11 UTC (permalink / raw)
To: Ignacy Gawedzki
Cc: Oleksij Rempel, ath9k-devel@lists.ath9k.org,
linux-wireless@vger.kernel.org
In-Reply-To: <20130619140905.GA14079@zenon.in.qult.net>
Btw, I'm a bit confused -- are you using ath9k_htc as per the subject,
or ath9k?
if it's ath9k_htc then I don't really understand why the patch that
*fixed* it for Corey *broke* it for you?
johannes
^ permalink raw reply
* Re: [ath9k-devel] ath9k_htc: station unable to authenticate
From: Ignacy Gawedzki @ 2013-06-19 14:09 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Berg, Johannes, ath9k-devel@lists.ath9k.org,
linux-wireless@vger.kernel.org
In-Reply-To: <51C1B467.1030509@rempel-privat.de>
On Wed, Jun 19, 2013 at 03:38:47PM +0200, thus spake Oleksij Rempel:
> hm.. looks like this code depends on power_save. is it correct?
>
> Ignacy,
>
> can you please take a look if power_save mode is enabled by you.
>
> iw dev wlan0 get power_save
>
> if yes. Try to disable it.
Although I have been building with CFG80211_DEFAULT_PS=y, the iw get
power_save command tells me it's off.
> I think last week power_save was set to disabled by default. There
> are too many bugs right now.
I'll test without CFG80211_DEFAULT_PS, just to be sure.
Thanks for the hint.
--
NO CARRIER
^ permalink raw reply
* Re: [PATCH] nl80211: fix attrbuf access race by allocating a separate one
From: John W. Linville @ 2013-06-19 13:51 UTC (permalink / raw)
To: David Miller; +Cc: johannes, torvalds, linux-wireless, netdev
In-Reply-To: <20130619.013900.786603036908799505.davem@davemloft.net>
On Wed, Jun 19, 2013 at 01:39:00AM -0700, David Miller wrote:
> From: Johannes Berg <johannes@sipsolutions.net>
> Date: Wed, 19 Jun 2013 10:23:58 +0200
>
> > From: Johannes Berg <johannes.berg@intel.com>
> >
> > Since my commit 3713b4e364, nl80211_dump_wiphy() uses the global
> > nl80211_fam.attrbuf for parsing the incoming data. This wouldn't
> > be a problem if it only did so on the first dump iteration which
> > is locked against other commands in generic netlink, but due to
> > space constraints in cb->args (the needed state doesn't fit) I
> > decided to always parse the original message. That's racy though
> > since nl80211_fam.attrbuf could be used by some other parsing in
> > generic netlink concurrently.
> >
> > For now, fix this by allocating a separate parse buffer (it's a
> > bit too big for the stack, currently 1448 bytes on 64-bit). For
> > -next, I'll change the code to parse into the global buffer in
> > the first round only and then allocate a smaller buffer to keep
> > the state in cb->args.
> >
> > Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>
> Acked-by: David S. Miller <davem@davemloft.net>
Acked-by: John W. Linville <linville@tuxdriver.com>
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* [PATCH 1/2] cfg80211: require passing BSS struct back to cfg80211_assoc_timeout
From: Johannes Berg @ 2013-06-19 13:53 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
Doing so will allow us to hold the BSS (not just ref it) over the
association process, thus ensuring that it doesn't time out and
gets invisible to the user (e.g. in 'iw wlan0 link'.)
This also fixes a leak in mac80211 where it doesn't always release
the BSS struct properly in all cases where calling this function.
This leak was reported by Ben Greear.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
include/net/cfg80211.h | 16 ++++++++--------
net/mac80211/mlme.c | 15 +++++++--------
net/wireless/mlme.c | 8 +++++---
3 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index e3a39fc..7b0730a 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1459,7 +1459,8 @@ const u8 *ieee80211_bss_get_ie(struct cfg80211_bss *bss, u8 ie);
* This structure provides information needed to complete IEEE 802.11
* authentication.
*
- * @bss: The BSS to authenticate with.
+ * @bss: The BSS to authenticate with, the callee must obtain a reference
+ * to it if it needs to keep it.
* @auth_type: Authentication type (algorithm)
* @ie: Extra IEs to add to Authentication frame or %NULL
* @ie_len: Length of ie buffer in octets
@@ -1497,11 +1498,10 @@ enum cfg80211_assoc_req_flags {
*
* This structure provides information needed to complete IEEE 802.11
* (re)association.
- * @bss: The BSS to associate with. If the call is successful the driver
- * is given a reference that it must release, normally via a call to
- * cfg80211_send_rx_assoc(), or, if association timed out, with a
- * call to cfg80211_put_bss() (in addition to calling
- * cfg80211_send_assoc_timeout())
+ * @bss: The BSS to associate with. If the call is successful the driver is
+ * given a reference that it must give back to cfg80211_send_rx_assoc()
+ * or to cfg80211_assoc_timeout(). To ensure proper refcounting, new
+ * association requests while already associating must be rejected.
* @ie: Extra IEs to add to (Re)Association Request frame or %NULL
* @ie_len: Length of ie buffer in octets
* @use_mfp: Use management frame protection (IEEE 802.11w) in this association
@@ -3522,11 +3522,11 @@ void cfg80211_rx_assoc_resp(struct net_device *dev,
/**
* cfg80211_assoc_timeout - notification of timed out association
* @dev: network device
- * @addr: The MAC address of the device with which the association timed out
+ * @bss: The BSS entry with which association timed out.
*
* This function may sleep. The caller must hold the corresponding wdev's mutex.
*/
-void cfg80211_assoc_timeout(struct net_device *dev, const u8 *addr);
+void cfg80211_assoc_timeout(struct net_device *dev, struct cfg80211_bss *bss);
/**
* cfg80211_tx_mlme_mgmt - notification of transmitted deauth/disassoc frame
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index e0939eb..08b3e71 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2722,8 +2722,7 @@ static void ieee80211_rx_mgmt_assoc_resp(struct ieee80211_sub_if_data *sdata,
if (!ieee80211_assoc_success(sdata, bss, mgmt, len)) {
/* oops -- internal error -- send timeout for now */
ieee80211_destroy_assoc_data(sdata, false);
- cfg80211_put_bss(sdata->local->hw.wiphy, bss);
- cfg80211_assoc_timeout(sdata->dev, mgmt->bssid);
+ cfg80211_assoc_timeout(sdata->dev, bss);
return;
}
sdata_info(sdata, "associated\n");
@@ -3440,13 +3439,10 @@ void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata)
time_after(jiffies, ifmgd->assoc_data->timeout)) {
if ((ifmgd->assoc_data->need_beacon && !ifmgd->have_beacon) ||
ieee80211_do_assoc(sdata)) {
- u8 bssid[ETH_ALEN];
-
- memcpy(bssid, ifmgd->assoc_data->bss->bssid, ETH_ALEN);
+ struct cfg80211_bss *bss = ifmgd->assoc_data->bss;
ieee80211_destroy_assoc_data(sdata, false);
-
- cfg80211_assoc_timeout(sdata->dev, bssid);
+ cfg80211_assoc_timeout(sdata->dev, bss);
}
} else if (ifmgd->assoc_data && ifmgd->assoc_data->timeout_started)
run_again(sdata, ifmgd->assoc_data->timeout);
@@ -4372,8 +4368,11 @@ void ieee80211_mgd_stop(struct ieee80211_sub_if_data *sdata)
cancel_work_sync(&ifmgd->chswitch_work);
sdata_lock(sdata);
- if (ifmgd->assoc_data)
+ if (ifmgd->assoc_data) {
+ struct cfg80211_bss *bss = ifmgd->assoc_data->bss;
ieee80211_destroy_assoc_data(sdata, false);
+ cfg80211_assoc_timeout(sdata->dev, bss);
+ }
if (ifmgd->auth_data)
ieee80211_destroy_auth_data(sdata, false);
del_timer_sync(&ifmgd->timer);
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index a61a44b..dd6f79d 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -131,16 +131,18 @@ void cfg80211_auth_timeout(struct net_device *dev, const u8 *addr)
}
EXPORT_SYMBOL(cfg80211_auth_timeout);
-void cfg80211_assoc_timeout(struct net_device *dev, const u8 *addr)
+void cfg80211_assoc_timeout(struct net_device *dev, struct cfg80211_bss *bss)
{
struct wireless_dev *wdev = dev->ieee80211_ptr;
struct wiphy *wiphy = wdev->wiphy;
struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
- trace_cfg80211_send_assoc_timeout(dev, addr);
+ trace_cfg80211_send_assoc_timeout(dev, bss->bssid);
- nl80211_send_assoc_timeout(rdev, dev, addr, GFP_KERNEL);
+ nl80211_send_assoc_timeout(rdev, dev, bss->bssid, GFP_KERNEL);
cfg80211_sme_assoc_timeout(wdev);
+
+ cfg80211_put_bss(wiphy, bss);
}
EXPORT_SYMBOL(cfg80211_assoc_timeout);
--
1.8.0
^ permalink raw reply related
* [PATCH 2/2] cfg80211: hold BSS over association process
From: Johannes Berg @ 2013-06-19 13:53 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg
In-Reply-To: <1371650017-14293-1-git-send-email-johannes@sipsolutions.net>
From: Johannes Berg <johannes.berg@intel.com>
This fixes the potential issue that the BSS struct that we use
and later assign to wdev->current_bss is removed from the scan
list while associating.
Also warn when we don't have a BSS struct in connect_result
unless it's from a driver that only has the connect() API.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/wireless/mlme.c | 4 ++++
net/wireless/sme.c | 15 ++++++++++-----
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index dd6f79d..bfac5e1 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -38,6 +38,7 @@ void cfg80211_rx_assoc_resp(struct net_device *dev, struct cfg80211_bss *bss,
* frame instead of reassoc.
*/
if (cfg80211_sme_rx_assoc_resp(wdev, status_code)) {
+ cfg80211_unhold_bss(bss_from_pub(bss));
cfg80211_put_bss(wiphy, bss);
return;
}
@@ -142,6 +143,7 @@ void cfg80211_assoc_timeout(struct net_device *dev, struct cfg80211_bss *bss)
nl80211_send_assoc_timeout(rdev, dev, bss->bssid, GFP_KERNEL);
cfg80211_sme_assoc_timeout(wdev);
+ cfg80211_unhold_bss(bss_from_pub(bss));
cfg80211_put_bss(wiphy, bss);
}
EXPORT_SYMBOL(cfg80211_assoc_timeout);
@@ -309,6 +311,8 @@ int cfg80211_mlme_assoc(struct cfg80211_registered_device *rdev,
goto out;
err = rdev_assoc(rdev, dev, req);
+ if (!err)
+ cfg80211_hold_bss(bss_from_pub(req->bss));
out:
if (err)
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index ae7e2cb..c0bf781 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -615,19 +615,24 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
kfree(wdev->connect_keys);
wdev->connect_keys = NULL;
wdev->ssid_len = 0;
- cfg80211_put_bss(wdev->wiphy, bss);
+ if (bss) {
+ cfg80211_unhold_bss(bss_from_pub(bss));
+ cfg80211_put_bss(wdev->wiphy, bss);
+ }
return;
}
- if (!bss)
+ if (!bss) {
+ WARN_ON_ONCE(!wiphy_to_dev(wdev->wiphy)->ops->connect);
bss = cfg80211_get_bss(wdev->wiphy, NULL, bssid,
wdev->ssid, wdev->ssid_len,
WLAN_CAPABILITY_ESS,
WLAN_CAPABILITY_ESS);
- if (WARN_ON(!bss))
- return;
+ if (WARN_ON(!bss))
+ return;
+ cfg80211_hold_bss(bss_from_pub(bss));
+ }
- cfg80211_hold_bss(bss_from_pub(bss));
wdev->current_bss = bss_from_pub(bss);
cfg80211_upload_connect_keys(wdev);
--
1.8.0
^ permalink raw reply related
* Re: [PATCH] nl80211: fix attrbuf access race by allocating a separate one
From: Sergei Shtylyov @ 2013-06-19 13:44 UTC (permalink / raw)
To: Johannes Berg
Cc: Linus Torvalds, David Miller, John Linville, Linux Wireless List,
Network Development
In-Reply-To: <1371630238.8349.6.camel@jlt4.sipsolutions.net>
Hello.
On 19-06-2013 12:23, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> Since my commit 3713b4e364, nl80211_dump_wiphy() uses the global
Please also specify that commit's summary line in parens.
> nl80211_fam.attrbuf for parsing the incoming data. This wouldn't
> be a problem if it only did so on the first dump iteration which
> is locked against other commands in generic netlink, but due to
> space constraints in cb->args (the needed state doesn't fit) I
> decided to always parse the original message. That's racy though
> since nl80211_fam.attrbuf could be used by some other parsing in
> generic netlink concurrently.
> For now, fix this by allocating a separate parse buffer (it's a
> bit too big for the stack, currently 1448 bytes on 64-bit). For
> -next, I'll change the code to parse into the global buffer in
> the first round only and then allocate a smaller buffer to keep
> the state in cb->args.
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
WBR, Sergei
^ permalink raw reply
* Re: [ath9k-devel] ath9k_htc: station unable to authenticate
From: Oleksij Rempel @ 2013-06-19 13:38 UTC (permalink / raw)
To: Berg, Johannes
Cc: Ignacy Gawedzki, ath9k-devel@lists.ath9k.org,
linux-wireless@vger.kernel.org
In-Reply-To: <1DC40B07CD6EC041A66726C271A73AE61AA312F5@IRSMSX102.ger.corp.intel.com>
Am 19.06.2013 15:08, schrieb Berg, Johannes:
>
>> mac80211: fix idle handling sequence
>>
>> Corey Richardson reported that my idle handling cleanup
>> (commit fd0f979a1b, "mac80211: simplify idle handling")
>> broke ath9k_htc. The reason appears to be that it wants
>> to go out of idle before switching channels. To fix it,
>> reimplement that sequence.
>>
>> Reported-by: Corey Richardson <corey@octayn.net>
>> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>>
>> Johannes, can you please take a look on it.
>
> Not very well, I don't know anything about ath9k. I'm willing to help out, but I can't really say what broke it.
hm.. looks like this code depends on power_save. is it correct?
Ignacy,
can you please take a look if power_save mode is enabled by you.
iw dev wlan0 get power_save
if yes. Try to disable it.
I think last week power_save was set to disabled by default. There are
too many bugs right now.
--
Regards,
Oleksij
^ permalink raw reply
* Re: bug-report: Ralink corp. Device [1814:539a] - Kernel 3.8 +
From: Stanislaw Gruszka @ 2013-06-19 13:12 UTC (permalink / raw)
To: Lars Zastrow; +Cc: backports, linux-wireless
In-Reply-To: <51BE8008.20302@gmx.de>
On Mon, Jun 17, 2013 at 05:18:32AM +0200, Lars Zastrow wrote:
> Dear Team,
>
> I'am using the following wireless-network-device:
>
> Network controller [0280]: Ralink corp. Device [1814:539a]
> Subsystem: Hewlett-Packard Company Device [103c:1839]
> Kernel driver in use: rt2800pci
>
> I had problems with it since I bought my laptop... The driver was
> unstable with the kernel I used back in autumn 2012. Howeverer, one
> kernel-update fixed it. Must be something about 3.4x?
>
> It was defenetly working with the Kernels from 3.5 till 3.7 on
> multible distros. 3.8 was bringing back the old Problems:
>
> - extremly low signal
> - hard to establish a connection to any acess-point.
>
> The last "problematic" kernel I have testet was 3.9.5-1 on
> Arch-Linux. Compiling the compat-drivers manually did not bring any
> fix.
>
> I am currently using 3.7.4-1 wich is working.
This most likely is a RT5390 TX power problem I made on 3.8.
This patch should help:
https://git.kernel.org/cgit/linux/kernel/git/linville/wireless.git/commit/?id=8c8d2017ba25c510ddf093419048460db1109bc4
Stanislaw
^ permalink raw reply
* RE: [ath9k-devel] ath9k_htc: station unable to authenticate
From: Berg, Johannes @ 2013-06-19 13:08 UTC (permalink / raw)
To: Oleksij Rempel, Ignacy Gawedzki, ath9k-devel@lists.ath9k.org
Cc: linux-wireless@vger.kernel.org
In-Reply-To: <51C1ACEC.2020703@rempel-privat.de>
IA0KPiAgICAgIG1hYzgwMjExOiBmaXggaWRsZSBoYW5kbGluZyBzZXF1ZW5jZQ0KPiANCj4gICAg
ICBDb3JleSBSaWNoYXJkc29uIHJlcG9ydGVkIHRoYXQgbXkgaWRsZSBoYW5kbGluZyBjbGVhbnVw
DQo+ICAgICAgKGNvbW1pdCBmZDBmOTc5YTFiLCAibWFjODAyMTE6IHNpbXBsaWZ5IGlkbGUgaGFu
ZGxpbmciKQ0KPiAgICAgIGJyb2tlIGF0aDlrX2h0Yy4gVGhlIHJlYXNvbiBhcHBlYXJzIHRvIGJl
IHRoYXQgaXQgd2FudHMNCj4gICAgICB0byBnbyBvdXQgb2YgaWRsZSBiZWZvcmUgc3dpdGNoaW5n
IGNoYW5uZWxzLiBUbyBmaXggaXQsDQo+ICAgICAgcmVpbXBsZW1lbnQgdGhhdCBzZXF1ZW5jZS4N
Cj4gDQo+ICAgICAgUmVwb3J0ZWQtYnk6IENvcmV5IFJpY2hhcmRzb24gPGNvcmV5QG9jdGF5bi5u
ZXQ+DQo+ICAgICAgU2lnbmVkLW9mZi1ieTogSm9oYW5uZXMgQmVyZyA8am9oYW5uZXMuYmVyZ0Bp
bnRlbC5jb20+DQo+IA0KPiBKb2hhbm5lcywgY2FuIHlvdSBwbGVhc2UgdGFrZSBhIGxvb2sgb24g
aXQuDQoNCk5vdCB2ZXJ5IHdlbGwsIEkgZG9uJ3Qga25vdyBhbnl0aGluZyBhYm91dCBhdGg5ay4g
SSdtIHdpbGxpbmcgdG8gaGVscCBvdXQsIGJ1dCBJIGNhbid0IHJlYWxseSBzYXkgd2hhdCBicm9r
ZSBpdC4NCg0Kam9oYW5uZXMNCg0KSW50ZWwgR21iSApEb3JuYWNoZXIgU3RyYXNzZSAxCjg1NjIy
IEZlbGRraXJjaGVuL011ZW5jaGVuLCBEZXV0c2NobGFuZApTaXR6IGRlciBHZXNlbGxzY2hhZnQ6
IEZlbGRraXJjaGVuIGJlaSBNdWVuY2hlbgpHZXNjaGFlZnRzZnVlaHJlcjogQ2hyaXN0aWFuIExh
bXByZWNodGVyLCBIYW5uZXMgU2Nod2FkZXJlciwgRG91Z2xhcyBMdXNrClJlZ2lzdGVyZ2VyaWNo
dDogTXVlbmNoZW4gSFJCIDQ3NDU2ClVzdC4tSWROci4vVkFUIFJlZ2lzdHJhdGlvbiBOby46IERF
MTI5Mzg1ODk1CkNpdGliYW5rIEZyYW5rZnVydCBhLk0uIChCTFogNTAyIDEwOSAwMCkgNjAwMTE5
MDUyCg==
^ permalink raw reply
* Re: 3.7.8/amd64 full interrupt hangs due to iwlwifi under big nfs copies out
From: Stanislaw Gruszka @ 2013-06-19 13:09 UTC (permalink / raw)
To: Johannes Berg
Cc: Eric Dumazet, Marc MERLIN, David Miller, Larry.Finger, bhutchings,
linux-wireless, netdev
In-Reply-To: <1371575087.22256.2.camel@jlt4.sipsolutions.net>
On Tue, Jun 18, 2013 at 07:04:47PM +0200, Johannes Berg wrote:
> On Tue, 2013-06-18 at 09:52 -0700, Eric Dumazet wrote:
>
> > > We'll be submitting a patch to make single pages default.
> >
> > Do you think the same change would be possible for
> > drivers/net/wireless/iwlegacy/4965-mac.c ?
>
> Yes, Stanislaw?
Yes, I'll post 4965 patch.
Thanks
Stanislaw
^ permalink raw reply
* Re: mac80211: impact of netif_carrier_off during roaming
From: Dan Williams @ 2013-06-19 13:06 UTC (permalink / raw)
To: m.fitzjohn; +Cc: linux-wireless
In-Reply-To: <004401ce6ccf$5b69e620$123db260$%fitzjohn@samsung.com>
On Wed, 2013-06-19 at 10:28 +0100, Mark Fitzjohn wrote:
> Hi
>
> I've noticed that during roaming triggered by detection of a significantly
> better AP in wpa_supplicant that mac80211 calls netif_carrier_off and then
> netif_carrier_on when associated with the new AP.
>
> Does this have any detrimental impact on applications using the wifi
> connection or is the assumption that as long as the roam happens in a
> reasonable time then applications should tolerate it?
>
> Just wondering if signalling the netif_carrier_off only when the current AP
> is lost and there is no immediate alternative AP for the network available
> was considered?
Carrier is more "can I send data the other side is expected to receive",
and that's what it's doing in mac80211. If you're roaming but not yet
associated to the new AP, you can't send any data, thus carrier is down.
Carrier for Wi-Fi is simply more complicated than ethernet because Wi-Fi
is more complicated. The current approach provides a good notification
of L2 connectivity, which maps well to the existing Ethernet semantics
and how stuff like DHCP works if it wants to renew addresses. Something
needs to provide this and carrier does that in a consistent way.
What you're probably looking for is indications of
connection/disconnection from an SSID, which is provided by
mac80211/cfg80211 and wpa_supplicant.
Dan
^ permalink raw reply
* [PATCH mac80211-next] brcm80211: fix null pointer access
From: Antonio Quartulli @ 2013-06-19 11:35 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Antonio Quartulli
From: Antonio Quartulli <antonio@open-mesh.com>
Do not unconditionally access the chan variable in
brcmf_cfg80211_mgmt_tx() as it may be NULL.
Use freq instead.
Introduced by c2ff8cad64233b539c71a27e2a6e324001143ef0
("brcm80211: make mgmt_tx in brcmfmac accept a NULL channel")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index 8bd256b..d316d62 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -3985,8 +3985,7 @@ brcmf_cfg80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
le16_to_cpu(action_frame->len));
brcmf_dbg(TRACE, "Action frame, cookie=%lld, len=%d, freq=%d\n",
- *cookie, le16_to_cpu(action_frame->len),
- chan->center_freq);
+ *cookie, le16_to_cpu(action_frame->len), freq);
ack = brcmf_p2p_send_action_frame(cfg, cfg_to_ndev(cfg),
af_params);
--
1.8.1.5
^ permalink raw reply related
* Re: brcm80211: make mgmt_tx in brcmfmac accept a NULL channel
From: Antonio Quartulli @ 2013-06-19 11:27 UTC (permalink / raw)
To: Dan Carpenter
Cc: linux-wireless@vger.kernel.org, brcm80211-dev-list@broadcom.com
In-Reply-To: <20130619111620.GA969@elgon.mountain>
[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]
Hello Dan,
On Wed, Jun 19, 2013 at 04:16:27AM -0700, Dan Carpenter wrote:
> Hello Antonio Quartulli,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch c2ff8cad6423: "brcm80211: make mgmt_tx in brcmfmac accept a
> NULL channel" from Jun 11, 2013, leads to the following Smatch
> complaint:
>
> drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c:4052 brcmf_cfg80211_mgmt_tx()
> error: we previously assumed 'chan' could be null (see line 4041)
>
> drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> 4040 */
> 4041 if (chan)
> ^^^^
> New check reflects that "chan" can now be NULL.
>
> 4042 freq = chan->center_freq;
> 4043 else
> 4044 brcmf_fil_cmd_int_get(vif->ifp, BRCMF_C_GET_CHANNEL,
> 4045 &freq);
> 4046 chan_nr = ieee80211_frequency_to_channel(freq);
> 4047 af_params->channel = cpu_to_le32(chan_nr);
> 4048
> 4049 memcpy(action_frame->data, &buf[DOT11_MGMT_HDR_LEN],
> 4050 le16_to_cpu(action_frame->len));
> 4051
> 4052 brcmf_dbg(TRACE, "Action frame, cookie=%lld, len=%d, freq=%d\n",
> 4053 *cookie, le16_to_cpu(action_frame->len),
> 4054 chan->center_freq);
> ^^^^^^
> This needs to be updated as well.
>
Thank you very much for spotting this. I'm going to send a patch fixing it.
(however the line numbers were shifted..)
Regards,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* re: brcm80211: make mgmt_tx in brcmfmac accept a NULL channel
From: Dan Carpenter @ 2013-06-19 11:16 UTC (permalink / raw)
To: antonio; +Cc: linux-wireless, brcm80211-dev-list
Hello Antonio Quartulli,
This is a semi-automatic email about new static checker warnings.
The patch c2ff8cad6423: "brcm80211: make mgmt_tx in brcmfmac accept a
NULL channel" from Jun 11, 2013, leads to the following Smatch
complaint:
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c:4052 brcmf_cfg80211_mgmt_tx()
error: we previously assumed 'chan' could be null (see line 4041)
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
4040 */
4041 if (chan)
^^^^
New check reflects that "chan" can now be NULL.
4042 freq = chan->center_freq;
4043 else
4044 brcmf_fil_cmd_int_get(vif->ifp, BRCMF_C_GET_CHANNEL,
4045 &freq);
4046 chan_nr = ieee80211_frequency_to_channel(freq);
4047 af_params->channel = cpu_to_le32(chan_nr);
4048
4049 memcpy(action_frame->data, &buf[DOT11_MGMT_HDR_LEN],
4050 le16_to_cpu(action_frame->len));
4051
4052 brcmf_dbg(TRACE, "Action frame, cookie=%lld, len=%d, freq=%d\n",
4053 *cookie, le16_to_cpu(action_frame->len),
4054 chan->center_freq);
^^^^^^
This needs to be updated as well.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH 2/6] mac80211: Fix bss ref leak.
From: Johannes Berg @ 2013-06-19 10:51 UTC (permalink / raw)
To: greearb; +Cc: linux-wireless
In-Reply-To: <1371638988.8349.11.camel@jlt4.sipsolutions.net>
On Wed, 2013-06-19 at 12:49 +0200, Johannes Berg wrote:
> I think we should do this differently and have mac80211 pass the BSS
> entry back to cfg80211_connect_result. That way, we can also hold it to
> make sure it doesn't expire while we're trying to connect, which is
> another corner case here ...
Oh, actually, mac80211 doesn't have this issue but all the other drivers
do.
johannes
^ permalink raw reply
* Re: [PATCH 2/6] mac80211: Fix bss ref leak.
From: Johannes Berg @ 2013-06-19 10:49 UTC (permalink / raw)
To: greearb; +Cc: linux-wireless
In-Reply-To: <1371593017-10985-2-git-send-email-greearb@candelatech.com>
I think we should do this differently and have mac80211 pass the BSS
entry back to cfg80211_connect_result. That way, we can also hold it to
make sure it doesn't expire while we're trying to connect, which is
another corner case here ...
I'll take a look at this, but that'll be in mac80211-next.
johannes
^ permalink raw reply
* Re: [PATCH 1/6] wireless: Add memory usage debugging.
From: Johannes Berg @ 2013-06-19 9:49 UTC (permalink / raw)
To: greearb; +Cc: linux-wireless
In-Reply-To: <1371593017-10985-1-git-send-email-greearb@candelatech.com>
On Tue, 2013-06-18 at 15:03 -0700, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> The bss objects are reference counted, and the ies
> are also tricky to keep track of. Add option to
> track allocation and freeing of the ies and bss objects,
> and add debugfs files to show the current objects.
I'm not really sure this is worth it -- you found the bug with
kmemleak() after all. Having very specific things for all of this
doesn't really seem worth it.
Either way though, I can't apply the patches since they won't apply. I
realize that you're working on 3.9, but you're going to have to send me
patches I can apply to one of my trees (and right now, not really
mac80211 but mac80211-next)
johannes
^ permalink raw reply
* mac80211: impact of netif_carrier_off during roaming
From: Mark Fitzjohn @ 2013-06-19 9:28 UTC (permalink / raw)
To: linux-wireless
Hi
I've noticed that during roaming triggered by detection of a significantly
better AP in wpa_supplicant that mac80211 calls netif_carrier_off and then
netif_carrier_on when associated with the new AP.
Does this have any detrimental impact on applications using the wifi
connection or is the assumption that as long as the roam happens in a
reasonable time then applications should tolerate it?
Just wondering if signalling the netif_carrier_off only when the current AP
is lost and there is no immediate alternative AP for the network available
was considered?
Thanks
Mark
^ permalink raw reply
* [PATCH] nl80211: use small state buffer for wiphy_dump
From: Johannes Berg @ 2013-06-19 9:00 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
Avoid parsing the original dump message again and again by
allocating a small state struct that is used by the functions
involved in the dump, storing this struct in cb->args[0].
This reduces the memory allocation size as well.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/wireless/nl80211.c | 210 +++++++++++++++++++++++++++----------------------
1 file changed, 116 insertions(+), 94 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index f8ffb9a..7dc3343 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1111,10 +1111,16 @@ nl80211_send_mgmt_stypes(struct sk_buff *msg,
return 0;
}
+struct nl80211_dump_wiphy_state {
+ s64 filter_wiphy;
+ long start;
+ long split_start, band_start, chan_start;
+ bool split;
+};
+
static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
struct sk_buff *msg, u32 portid, u32 seq,
- int flags, bool split, long *split_start,
- long *band_start, long *chan_start)
+ int flags, struct nl80211_dump_wiphy_state *state)
{
void *hdr;
struct nlattr *nl_bands, *nl_band;
@@ -1125,19 +1131,14 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
int i;
const struct ieee80211_txrx_stypes *mgmt_stypes =
dev->wiphy.mgmt_stypes;
- long start = 0, start_chan = 0, start_band = 0;
u32 features;
hdr = nl80211hdr_put(msg, portid, seq, flags, NL80211_CMD_NEW_WIPHY);
if (!hdr)
return -ENOBUFS;
- /* allow always using the variables */
- if (!split) {
- split_start = &start;
- band_start = &start_band;
- chan_start = &start_chan;
- }
+ if (WARN_ON(!state))
+ return -EINVAL;
if (nla_put_u32(msg, NL80211_ATTR_WIPHY, dev->wiphy_idx) ||
nla_put_string(msg, NL80211_ATTR_WIPHY_NAME,
@@ -1146,7 +1147,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
cfg80211_rdev_list_generation))
goto nla_put_failure;
- switch (*split_start) {
+ switch (state->split_start) {
case 0:
if (nla_put_u8(msg, NL80211_ATTR_WIPHY_RETRY_SHORT,
dev->wiphy.retry_short) ||
@@ -1192,8 +1193,8 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
nla_put_flag(msg, WIPHY_FLAG_SUPPORTS_5_10_MHZ))
goto nla_put_failure;
- (*split_start)++;
- if (split)
+ state->split_start++;
+ if (state->split)
break;
case 1:
if (nla_put(msg, NL80211_ATTR_CIPHER_SUITES,
@@ -1237,22 +1238,23 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
}
}
- (*split_start)++;
- if (split)
+ state->split_start++;
+ if (state->split)
break;
case 2:
if (nl80211_put_iftypes(msg, NL80211_ATTR_SUPPORTED_IFTYPES,
dev->wiphy.interface_modes))
goto nla_put_failure;
- (*split_start)++;
- if (split)
+ state->split_start++;
+ if (state->split)
break;
case 3:
nl_bands = nla_nest_start(msg, NL80211_ATTR_WIPHY_BANDS);
if (!nl_bands)
goto nla_put_failure;
- for (band = *band_start; band < IEEE80211_NUM_BANDS; band++) {
+ for (band = state->band_start;
+ band < IEEE80211_NUM_BANDS; band++) {
struct ieee80211_supported_band *sband;
sband = dev->wiphy.bands[band];
@@ -1264,12 +1266,12 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
if (!nl_band)
goto nla_put_failure;
- switch (*chan_start) {
+ switch (state->chan_start) {
case 0:
if (nl80211_send_band_rateinfo(msg, sband))
goto nla_put_failure;
- (*chan_start)++;
- if (split)
+ state->chan_start++;
+ if (state->split)
break;
default:
/* add frequencies */
@@ -1278,7 +1280,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
if (!nl_freqs)
goto nla_put_failure;
- for (i = *chan_start - 1;
+ for (i = state->chan_start - 1;
i < sband->n_channels;
i++) {
nl_freq = nla_nest_start(msg, i);
@@ -1287,26 +1289,27 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
chan = &sband->channels[i];
- if (nl80211_msg_put_channel(msg, chan,
- split))
+ if (nl80211_msg_put_channel(
+ msg, chan,
+ state->split))
goto nla_put_failure;
nla_nest_end(msg, nl_freq);
- if (split)
+ if (state->split)
break;
}
if (i < sband->n_channels)
- *chan_start = i + 2;
+ state->chan_start = i + 2;
else
- *chan_start = 0;
+ state->chan_start = 0;
nla_nest_end(msg, nl_freqs);
}
nla_nest_end(msg, nl_band);
- if (split) {
+ if (state->split) {
/* start again here */
- if (*chan_start)
+ if (state->chan_start)
band--;
break;
}
@@ -1314,14 +1317,14 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
nla_nest_end(msg, nl_bands);
if (band < IEEE80211_NUM_BANDS)
- *band_start = band + 1;
+ state->band_start = band + 1;
else
- *band_start = 0;
+ state->band_start = 0;
/* if bands & channels are done, continue outside */
- if (*band_start == 0 && *chan_start == 0)
- (*split_start)++;
- if (split)
+ if (state->band_start == 0 && state->chan_start == 0)
+ state->split_start++;
+ if (state->split)
break;
case 4:
nl_cmds = nla_nest_start(msg, NL80211_ATTR_SUPPORTED_COMMANDS);
@@ -1387,7 +1390,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
}
CMD(start_p2p_device, START_P2P_DEVICE);
CMD(set_mcast_rate, SET_MCAST_RATE);
- if (split) {
+ if (state->split) {
CMD(crit_proto_start, CRIT_PROTOCOL_START);
CMD(crit_proto_stop, CRIT_PROTOCOL_STOP);
}
@@ -1411,8 +1414,8 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
}
nla_nest_end(msg, nl_cmds);
- (*split_start)++;
- if (split)
+ state->split_start++;
+ if (state->split)
break;
case 5:
if (dev->ops->remain_on_channel &&
@@ -1428,29 +1431,30 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
if (nl80211_send_mgmt_stypes(msg, mgmt_stypes))
goto nla_put_failure;
- (*split_start)++;
- if (split)
+ state->split_start++;
+ if (state->split)
break;
case 6:
#ifdef CONFIG_PM
- if (nl80211_send_wowlan(msg, dev, split))
+ if (nl80211_send_wowlan(msg, dev, state->split))
goto nla_put_failure;
- (*split_start)++;
- if (split)
+ state->split_start++;
+ if (state->split)
break;
#else
- (*split_start)++;
+ state->split_start++;
#endif
case 7:
if (nl80211_put_iftypes(msg, NL80211_ATTR_SOFTWARE_IFTYPES,
dev->wiphy.software_iftypes))
goto nla_put_failure;
- if (nl80211_put_iface_combinations(&dev->wiphy, msg, split))
+ if (nl80211_put_iface_combinations(&dev->wiphy, msg,
+ state->split))
goto nla_put_failure;
- (*split_start)++;
- if (split)
+ state->split_start++;
+ if (state->split)
break;
case 8:
if ((dev->wiphy.flags & WIPHY_FLAG_HAVE_AP_SME) &&
@@ -1464,7 +1468,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
* dump is split, otherwise it makes it too big. Therefore
* only advertise it in that case.
*/
- if (split)
+ if (state->split)
features |= NL80211_FEATURE_ADVERTISE_CHAN_LIMITS;
if (nla_put_u32(msg, NL80211_ATTR_FEATURE_FLAGS, features))
goto nla_put_failure;
@@ -1491,7 +1495,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
* case we'll continue with more data in the next round,
* but break unconditionally so unsplit data stops here.
*/
- (*split_start)++;
+ state->split_start++;
break;
case 9:
if (dev->wiphy.extended_capabilities &&
@@ -1510,7 +1514,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
goto nla_put_failure;
/* done */
- *split_start = 0;
+ state->split_start = 0;
break;
}
return genlmsg_end(msg, hdr);
@@ -1520,66 +1524,76 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
return -EMSGSIZE;
}
+static int nl80211_dump_wiphy_parse(struct sk_buff *skb,
+ struct netlink_callback *cb,
+ struct nl80211_dump_wiphy_state *state)
+{
+ struct nlattr **tb = nl80211_fam.attrbuf;
+ int ret = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
+ tb, nl80211_fam.maxattr, nl80211_policy);
+ /* ignore parse errors for backward compatibility */
+ if (ret)
+ return 0;
+
+ state->split = tb[NL80211_ATTR_SPLIT_WIPHY_DUMP];
+ if (tb[NL80211_ATTR_WIPHY])
+ state->filter_wiphy = nla_get_u32(tb[NL80211_ATTR_WIPHY]);
+ if (tb[NL80211_ATTR_WDEV])
+ state->filter_wiphy = nla_get_u64(tb[NL80211_ATTR_WDEV]) >> 32;
+ if (tb[NL80211_ATTR_IFINDEX]) {
+ struct net_device *netdev;
+ struct cfg80211_registered_device *rdev;
+ int ifidx = nla_get_u32(tb[NL80211_ATTR_IFINDEX]);
+
+ netdev = dev_get_by_index(sock_net(skb->sk), ifidx);
+ if (!netdev)
+ return -ENODEV;
+ if (netdev->ieee80211_ptr) {
+ rdev = wiphy_to_dev(
+ netdev->ieee80211_ptr->wiphy);
+ state->filter_wiphy = rdev->wiphy_idx;
+ }
+ dev_put(netdev);
+ }
+
+ return 0;
+}
+
static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
{
int idx = 0, ret;
- int start = cb->args[0];
+ struct nl80211_dump_wiphy_state *state = (void *)cb->args[0];
struct cfg80211_registered_device *dev;
- s64 filter_wiphy = -1;
- bool split = false;
- struct nlattr **tb;
- int res;
-
- /* will be zeroed in nlmsg_parse() */
- tb = kmalloc(sizeof(*tb) * (NL80211_ATTR_MAX + 1), GFP_KERNEL);
- if (!tb)
- return -ENOMEM;
rtnl_lock();
- res = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
- tb, NL80211_ATTR_MAX, nl80211_policy);
- if (res == 0) {
- split = tb[NL80211_ATTR_SPLIT_WIPHY_DUMP];
- if (tb[NL80211_ATTR_WIPHY])
- filter_wiphy = nla_get_u32(tb[NL80211_ATTR_WIPHY]);
- if (tb[NL80211_ATTR_WDEV])
- filter_wiphy = nla_get_u64(tb[NL80211_ATTR_WDEV]) >> 32;
- if (tb[NL80211_ATTR_IFINDEX]) {
- struct net_device *netdev;
- int ifidx = nla_get_u32(tb[NL80211_ATTR_IFINDEX]);
-
- netdev = dev_get_by_index(sock_net(skb->sk), ifidx);
- if (!netdev) {
- rtnl_unlock();
- kfree(tb);
- return -ENODEV;
- }
- if (netdev->ieee80211_ptr) {
- dev = wiphy_to_dev(
- netdev->ieee80211_ptr->wiphy);
- filter_wiphy = dev->wiphy_idx;
- }
- dev_put(netdev);
+ if (!state) {
+ state = kzalloc(sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return -ENOMEM;
+ state->filter_wiphy = -1;
+ ret = nl80211_dump_wiphy_parse(skb, cb, state);
+ if (ret) {
+ kfree(state);
+ rtnl_unlock();
+ return ret;
}
+ cb->args[0] = (long)state;
}
- kfree(tb);
list_for_each_entry(dev, &cfg80211_rdev_list, list) {
if (!net_eq(wiphy_net(&dev->wiphy), sock_net(skb->sk)))
continue;
- if (++idx <= start)
+ if (++idx <= state->start)
continue;
- if (filter_wiphy != -1 && dev->wiphy_idx != filter_wiphy)
+ if (state->filter_wiphy != -1 &&
+ state->filter_wiphy != dev->wiphy_idx)
continue;
/* attempt to fit multiple wiphy data chunks into the skb */
do {
ret = nl80211_send_wiphy(dev, skb,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
- NLM_F_MULTI,
- split, &cb->args[1],
- &cb->args[2],
- &cb->args[3]);
+ NLM_F_MULTI, state);
if (ret < 0) {
/*
* If sending the wiphy data didn't fit (ENOBUFS
@@ -1604,27 +1618,34 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
idx--;
break;
}
- } while (cb->args[1] > 0);
+ } while (state->split_start > 0);
break;
}
rtnl_unlock();
- cb->args[0] = idx;
+ state->start = idx;
return skb->len;
}
+static int nl80211_dump_wiphy_done(struct netlink_callback *cb)
+{
+ kfree((void *)cb->args[0]);
+ return 0;
+}
+
static int nl80211_get_wiphy(struct sk_buff *skb, struct genl_info *info)
{
struct sk_buff *msg;
struct cfg80211_registered_device *dev = info->user_ptr[0];
+ struct nl80211_dump_wiphy_state state = {};
msg = nlmsg_new(4096, GFP_KERNEL);
if (!msg)
return -ENOMEM;
if (nl80211_send_wiphy(dev, msg, info->snd_portid, info->snd_seq, 0,
- false, NULL, NULL, NULL) < 0) {
+ &state) < 0) {
nlmsg_free(msg);
return -ENOBUFS;
}
@@ -8418,6 +8439,7 @@ static struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_GET_WIPHY,
.doit = nl80211_get_wiphy,
.dumpit = nl80211_dump_wiphy,
+ .done = nl80211_dump_wiphy_done,
.policy = nl80211_policy,
/* can be retrieved by unprivileged users */
.internal_flags = NL80211_FLAG_NEED_WIPHY |
@@ -9038,13 +9060,13 @@ static struct genl_multicast_group nl80211_regulatory_mcgrp = {
void nl80211_notify_dev_rename(struct cfg80211_registered_device *rdev)
{
struct sk_buff *msg;
+ struct nl80211_dump_wiphy_state state = {};
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
if (!msg)
return;
- if (nl80211_send_wiphy(rdev, msg, 0, 0, 0,
- false, NULL, NULL, NULL) < 0) {
+ if (nl80211_send_wiphy(rdev, msg, 0, 0, 0, &state) < 0) {
nlmsg_free(msg);
return;
}
--
1.8.0
^ permalink raw reply related
* Re: [PATCH] nl80211: fix attrbuf access race by allocating a separate one
From: David Miller @ 2013-06-19 8:39 UTC (permalink / raw)
To: johannes; +Cc: torvalds, linville, linux-wireless, netdev
In-Reply-To: <1371630238.8349.6.camel@jlt4.sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Wed, 19 Jun 2013 10:23:58 +0200
> From: Johannes Berg <johannes.berg@intel.com>
>
> Since my commit 3713b4e364, nl80211_dump_wiphy() uses the global
> nl80211_fam.attrbuf for parsing the incoming data. This wouldn't
> be a problem if it only did so on the first dump iteration which
> is locked against other commands in generic netlink, but due to
> space constraints in cb->args (the needed state doesn't fit) I
> decided to always parse the original message. That's racy though
> since nl80211_fam.attrbuf could be used by some other parsing in
> generic netlink concurrently.
>
> For now, fix this by allocating a separate parse buffer (it's a
> bit too big for the stack, currently 1448 bytes on 64-bit). For
> -next, I'll change the code to parse into the global buffer in
> the first round only and then allocate a smaller buffer to keep
> the state in cb->args.
>
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* [PATCH] nl80211: fix attrbuf access race by allocating a separate one
From: Johannes Berg @ 2013-06-19 8:23 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Miller, John Linville, Linux Wireless List,
Network Development
In-Reply-To: <1371628488.8349.3.camel@jlt4.sipsolutions.net>
From: Johannes Berg <johannes.berg@intel.com>
Since my commit 3713b4e364, nl80211_dump_wiphy() uses the global
nl80211_fam.attrbuf for parsing the incoming data. This wouldn't
be a problem if it only did so on the first dump iteration which
is locked against other commands in generic netlink, but due to
space constraints in cb->args (the needed state doesn't fit) I
decided to always parse the original message. That's racy though
since nl80211_fam.attrbuf could be used by some other parsing in
generic netlink concurrently.
For now, fix this by allocating a separate parse buffer (it's a
bit too big for the stack, currently 1448 bytes on 64-bit). For
-next, I'll change the code to parse into the global buffer in
the first round only and then allocate a smaller buffer to keep
the state in cb->args.
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
Let me know if you want to apply this directly, otherwise I'll send it
on its way to John.
net/wireless/nl80211.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d5aed3b..b14b7e3 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1564,12 +1564,17 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
struct cfg80211_registered_device *dev;
s64 filter_wiphy = -1;
bool split = false;
- struct nlattr **tb = nl80211_fam.attrbuf;
+ struct nlattr **tb;
int res;
+ /* will be zeroed in nlmsg_parse() */
+ tb = kmalloc(sizeof(*tb) * (NL80211_ATTR_MAX + 1), GFP_KERNEL);
+ if (!tb)
+ return -ENOMEM;
+
mutex_lock(&cfg80211_mutex);
res = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
- tb, nl80211_fam.maxattr, nl80211_policy);
+ tb, NL80211_ATTR_MAX, nl80211_policy);
if (res == 0) {
split = tb[NL80211_ATTR_SPLIT_WIPHY_DUMP];
if (tb[NL80211_ATTR_WIPHY])
@@ -1583,6 +1588,7 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
netdev = dev_get_by_index(sock_net(skb->sk), ifidx);
if (!netdev) {
mutex_unlock(&cfg80211_mutex);
+ kfree(tb);
return -ENODEV;
}
if (netdev->ieee80211_ptr) {
@@ -1593,6 +1599,7 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
dev_put(netdev);
}
}
+ kfree(tb);
list_for_each_entry(dev, &cfg80211_rdev_list, list) {
if (!net_eq(wiphy_net(&dev->wiphy), sock_net(skb->sk)))
--
1.8.0
^ permalink raw reply related
* Re: nl80211 NULL pointer dereference
From: Johannes Berg @ 2013-06-19 7:54 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Miller, John Linville, Linux Wireless List,
Network Development
In-Reply-To: <CA+55aFz=eG5-zh1toHxrx=4Qm4DvwyiBCU3u-6tc0-utfZ6xiA@mail.gmail.com>
On Tue, 2013-06-18 at 16:24 -1000, Linus Torvalds wrote:
> So it would seem that it's that
>
> info->attrs[NL80211_ATTR_REG_RULES]
>
> thing that is NULL.
>
> And yes, the code checks that for being non-NULL in at the top of the
> function, but maybe there is a race with something else setting it to
> NULL? There is a kzalloc(GFP_KERNEL) in between, so it doesn't even
> have to be a very small race...
Yes. I looked at it, and reproduced it (after making the window larger
by putting some sleeps in there and WARN_ON()). It's really just a
stupid mistake I made: in nl80211_dump_wiphy() I parse attributes into
the global nl80211_fam.attrbuf, without making sure that it has proper
locking. Normally we do something like that only on the first iteration
of a dump which is OK because it's locked, but here I did it always,
which is clearly a bug.
I'll have a patch in a minute.
johannes
^ permalink raw reply
* Re: nl80211 NULL pointer dereference
From: David Miller @ 2013-06-19 7:47 UTC (permalink / raw)
To: torvalds; +Cc: johannes, linville, linux-wireless, netdev
In-Reply-To: <CA+55aFz=eG5-zh1toHxrx=4Qm4DvwyiBCU3u-6tc0-utfZ6xiA@mail.gmail.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Tue, 18 Jun 2013 16:24:57 -1000
> And yes, the code checks that for being non-NULL in at the top of the
> function, but maybe there is a race with something else setting it to
> NULL? There is a kzalloc(GFP_KERNEL) in between, so it doesn't even
> have to be a very small race...
The nl80211 code uses a flag for each netlink command to determine
whether the RTNL mutex should be held across the operation.
This is handled in the pre_doit and post_doit methods implemented
in nl80211.c.
And this operation, in fact, just so happens to be one that doesn't
have the "take the RTNL mutex" flag set.
But for internal consistency of the netlink message itself, the RTNL
mutex should not matter. It's in a private SKB buffer which is in use
only by the ->doit() method.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox