2011-10-27 12:45:12

by Johannes Berg

[permalink] [raw]
Subject: [PATCH] cfg80211: annotate cfg80211_inform_bss

From: Johannes Berg <[email protected]>

This function returns a referenced BSS struct
(or NULL), annotate with __must_check. It seems
that a lot of drivers get this completely wrong
and leak all BSS structs as a result.

Reported-by: Adam Mikuta <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
CC lots of driver maintainers who need to fix this bug.

include/net/cfg80211.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

--- a/include/net/cfg80211.h 2011-10-27 14:05:17.000000000 +0200
+++ b/include/net/cfg80211.h 2011-10-27 14:42:51.000000000 +0200
@@ -2651,8 +2651,10 @@ void cfg80211_sched_scan_stopped(struct
*
* This informs cfg80211 that BSS information was found and
* the BSS should be updated/added.
+ *
+ * NOTE: Returns a referenced struct, must be released with cfg80211_put_bss()!
*/
-struct cfg80211_bss*
+struct cfg80211_bss * __must_check
cfg80211_inform_bss_frame(struct wiphy *wiphy,
struct ieee80211_channel *channel,
struct ieee80211_mgmt *mgmt, size_t len,
@@ -2674,8 +2676,10 @@ cfg80211_inform_bss_frame(struct wiphy *
*
* This informs cfg80211 that BSS information was found and
* the BSS should be updated/added.
+ *
+ * NOTE: Returns a referenced struct, must be released with cfg80211_put_bss()!
*/
-struct cfg80211_bss*
+struct cfg80211_bss * __must_check
cfg80211_inform_bss(struct wiphy *wiphy,
struct ieee80211_channel *channel,
const u8 *bssid,




2011-10-28 05:06:32

by Jussi Kivilinna

[permalink] [raw]
Subject: [PATCH] rndis_wlan: release BSS structures returned by cfg80211_inform_bss()

Patch fixes rndis_wlan to release referenced BSS structure returned by
cfg80211_inform_bss().

Signed-off-by: Jussi Kivilinna <[email protected]>
---
drivers/net/wireless/rndis_wlan.c | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index 15464d5..9cc4b8c 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -1972,11 +1972,12 @@ static int rndis_scan(struct wiphy *wiphy, struct net_device *dev,
return ret;
}

-static struct cfg80211_bss *rndis_bss_info_update(struct usbnet *usbdev,
- struct ndis_80211_bssid_ex *bssid)
+static bool rndis_bss_info_update(struct usbnet *usbdev,
+ struct ndis_80211_bssid_ex *bssid)
{
struct rndis_wlan_private *priv = get_rndis_wlan_priv(usbdev);
struct ieee80211_channel *channel;
+ struct cfg80211_bss *bss;
s32 signal;
u64 timestamp;
u16 capability;
@@ -2015,9 +2016,12 @@ static struct cfg80211_bss *rndis_bss_info_update(struct usbnet *usbdev,
capability = le16_to_cpu(fixed->capabilities);
beacon_interval = le16_to_cpu(fixed->beacon_interval);

- return cfg80211_inform_bss(priv->wdev.wiphy, channel, bssid->mac,
+ bss = cfg80211_inform_bss(priv->wdev.wiphy, channel, bssid->mac,
timestamp, capability, beacon_interval, ie, ie_len, signal,
GFP_KERNEL);
+ cfg80211_put_bss(bss);
+
+ return (bss != NULL);
}

static struct ndis_80211_bssid_ex *next_bssid_list_item(
@@ -2641,6 +2645,7 @@ static void rndis_wlan_craft_connected_bss(struct usbnet *usbdev, u8 *bssid,
struct ieee80211_channel *channel;
struct ndis_80211_conf config;
struct ndis_80211_ssid ssid;
+ struct cfg80211_bss *bss;
s32 signal;
u64 timestamp;
u16 capability;
@@ -2714,9 +2719,10 @@ static void rndis_wlan_craft_connected_bss(struct usbnet *usbdev, u8 *bssid,
bssid, (u32)timestamp, capability, beacon_interval, ie_len,
ssid.essid, signal);

- cfg80211_inform_bss(priv->wdev.wiphy, channel, bssid,
+ bss = cfg80211_inform_bss(priv->wdev.wiphy, channel, bssid,
timestamp, capability, beacon_interval, ie_buf, ie_len,
signal, GFP_KERNEL);
+ cfg80211_put_bss(bss);
}

/*


2011-10-28 11:48:12

by Dave Kilroy

[permalink] [raw]
Subject: [PATCH v2] orinoco: release BSS structures returned by cfg80211_inform_bss()

The pointer returned by cfg80211_inform_bss is a referenced
struct. The orinoco driver does not need to keep the struct, so
we just release it.

Signed-off-by: David Kilroy <[email protected]>
---
v2: Avoid unnecessary NULL checks
---
drivers/net/wireless/orinoco/scan.c | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/orinoco/scan.c b/drivers/net/wireless/orinoco/scan.c
index e99ca1c..96e39ed 100644
--- a/drivers/net/wireless/orinoco/scan.c
+++ b/drivers/net/wireless/orinoco/scan.c
@@ -76,6 +76,7 @@ static void orinoco_add_hostscan_result(struct orinoco_private *priv,
{
struct wiphy *wiphy = priv_to_wiphy(priv);
struct ieee80211_channel *channel;
+ struct cfg80211_bss *cbss;
u8 *ie;
u8 ie_buf[46];
u64 timestamp;
@@ -121,9 +122,10 @@ static void orinoco_add_hostscan_result(struct orinoco_private *priv,
beacon_interval = le16_to_cpu(bss->a.beacon_interv);
signal = SIGNAL_TO_MBM(le16_to_cpu(bss->a.level));

- cfg80211_inform_bss(wiphy, channel, bss->a.bssid, timestamp,
- capability, beacon_interval, ie_buf, ie_len,
- signal, GFP_KERNEL);
+ cbss = cfg80211_inform_bss(wiphy, channel, bss->a.bssid, timestamp,
+ capability, beacon_interval, ie_buf, ie_len,
+ signal, GFP_KERNEL);
+ cfg80211_put_bss(cbss);
}

void orinoco_add_extscan_result(struct orinoco_private *priv,
@@ -132,6 +134,7 @@ void orinoco_add_extscan_result(struct orinoco_private *priv,
{
struct wiphy *wiphy = priv_to_wiphy(priv);
struct ieee80211_channel *channel;
+ struct cfg80211_bss *cbss;
const u8 *ie;
u64 timestamp;
s32 signal;
@@ -152,9 +155,10 @@ void orinoco_add_extscan_result(struct orinoco_private *priv,
ie = bss->data;
signal = SIGNAL_TO_MBM(bss->level);

- cfg80211_inform_bss(wiphy, channel, bss->bssid, timestamp,
- capability, beacon_interval, ie, ie_len,
- signal, GFP_KERNEL);
+ cbss = cfg80211_inform_bss(wiphy, channel, bss->bssid, timestamp,
+ capability, beacon_interval, ie, ie_len,
+ signal, GFP_KERNEL);
+ cfg80211_put_bss(cbss);
}

void orinoco_add_hostscan_results(struct orinoco_private *priv,
--
1.7.4.1


2011-10-28 10:09:53

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] orinoco: release BSS structures returned by cfg80211_inform_bss()

On Fri, 2011-10-28 at 11:00 +0100, David Kilroy wrote:
> The pointer returned by cfg80211_inform_bss is a referenced
> struct. The orinoco driver does not need to keep the struct, so
> we just release it.

Thanks!


> + cbss = cfg80211_inform_bss(wiphy, channel, bss->a.bssid, timestamp,
> + capability, beacon_interval, ie_buf, ie_len,
> + signal, GFP_KERNEL);
> + if (cbss)
> + cfg80211_put_bss(cbss);

Obviously doesn't hurt, but cfg80211_put_bss(NULL) is acceptable, if
you'd prefer the code that way.

johannes


2011-10-27 19:42:44

by Bing Zhao

[permalink] [raw]
Subject: RE: [PATCH] cfg80211: annotate cfg80211_inform_bss

SGkgSm9oYW5uZXMsDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogbGlu
dXgtd2lyZWxlc3Mtb3duZXJAdmdlci5rZXJuZWwub3JnIFttYWlsdG86bGludXgtd2lyZWxlc3Mt
b3duZXJAdmdlci5rZXJuZWwub3JnXSBPbiBCZWhhbGYgT2YNCj4gSm9oYW5uZXMgQmVyZw0KPiBT
ZW50OiBUaHVyc2RheSwgT2N0b2JlciAyNywgMjAxMSA1OjQ1IEFNDQo+IFRvOiBKb2huIExpbnZp
bGxlDQo+IENjOiBsaW51eC13aXJlbGVzczsgU2FtdWVsIE9ydGl6OyBCaW5nIFpoYW87IEp1c3Np
IEtpdmlsaW5uYTsgRGFuIFdpbGxpYW1zOyBBcmVuZCBWYW4gU3ByaWVsDQo+IFN1YmplY3Q6IFtQ
QVRDSF0gY2ZnODAyMTE6IGFubm90YXRlIGNmZzgwMjExX2luZm9ybV9ic3MNCj4gDQo+IEZyb206
IEpvaGFubmVzIEJlcmcgPGpvaGFubmVzLmJlcmdAaW50ZWwuY29tPg0KPiANCj4gVGhpcyBmdW5j
dGlvbiByZXR1cm5zIGEgcmVmZXJlbmNlZCBCU1Mgc3RydWN0DQo+IChvciBOVUxMKSwgYW5ub3Rh
dGUgd2l0aCBfX211c3RfY2hlY2suIEl0IHNlZW1zDQo+IHRoYXQgYSBsb3Qgb2YgZHJpdmVycyBn
ZXQgdGhpcyBjb21wbGV0ZWx5IHdyb25nDQo+IGFuZCBsZWFrIGFsbCBCU1Mgc3RydWN0cyBhcyBh
IHJlc3VsdC4NCj4gDQo+IFJlcG9ydGVkLWJ5OiBBZGFtIE1pa3V0YSA8QWRhbS5NaWt1dGFAdGll
dG8uY29tPg0KPiBTaWduZWQtb2ZmLWJ5OiBKb2hhbm5lcyBCZXJnIDxqb2hhbm5lcy5iZXJnQGlu
dGVsLmNvbT4NCj4gLS0tDQo+IENDIGxvdHMgb2YgZHJpdmVyIG1haW50YWluZXJzIHdobyBuZWVk
IHRvIGZpeCB0aGlzIGJ1Zy4NCg0KSSB3aWxsIHdvcmsgb24gdGhlIG13aWZpZXggZHJpdmVyIHBh
cnQuDQoNClRoYW5rcywNCkJpbmcNCg0KPiANCj4gIGluY2x1ZGUvbmV0L2NmZzgwMjExLmggfCAg
ICA4ICsrKysrKy0tDQo+ICAxIGZpbGUgY2hhbmdlZCwgNiBpbnNlcnRpb25zKCspLCAyIGRlbGV0
aW9ucygtKQ0KPiANCj4gLS0tIGEvaW5jbHVkZS9uZXQvY2ZnODAyMTEuaAkyMDExLTEwLTI3IDE0
OjA1OjE3LjAwMDAwMDAwMCArMDIwMA0KPiArKysgYi9pbmNsdWRlL25ldC9jZmc4MDIxMS5oCTIw
MTEtMTAtMjcgMTQ6NDI6NTEuMDAwMDAwMDAwICswMjAwDQo+IEBAIC0yNjUxLDggKzI2NTEsMTAg
QEAgdm9pZCBjZmc4MDIxMV9zY2hlZF9zY2FuX3N0b3BwZWQoc3RydWN0DQo+ICAgKg0KPiAgICog
VGhpcyBpbmZvcm1zIGNmZzgwMjExIHRoYXQgQlNTIGluZm9ybWF0aW9uIHdhcyBmb3VuZCBhbmQN
Cj4gICAqIHRoZSBCU1Mgc2hvdWxkIGJlIHVwZGF0ZWQvYWRkZWQuDQo+ICsgKg0KPiArICogTk9U
RTogUmV0dXJucyBhIHJlZmVyZW5jZWQgc3RydWN0LCBtdXN0IGJlIHJlbGVhc2VkIHdpdGggY2Zn
ODAyMTFfcHV0X2JzcygpIQ0KPiAgICovDQo+IC1zdHJ1Y3QgY2ZnODAyMTFfYnNzKg0KPiArc3Ry
dWN0IGNmZzgwMjExX2JzcyAqIF9fbXVzdF9jaGVjaw0KPiAgY2ZnODAyMTFfaW5mb3JtX2Jzc19m
cmFtZShzdHJ1Y3Qgd2lwaHkgKndpcGh5LA0KPiAgCQkJICBzdHJ1Y3QgaWVlZTgwMjExX2NoYW5u
ZWwgKmNoYW5uZWwsDQo+ICAJCQkgIHN0cnVjdCBpZWVlODAyMTFfbWdtdCAqbWdtdCwgc2l6ZV90
IGxlbiwNCj4gQEAgLTI2NzQsOCArMjY3NiwxMCBAQCBjZmc4MDIxMV9pbmZvcm1fYnNzX2ZyYW1l
KHN0cnVjdCB3aXBoeSAqDQo+ICAgKg0KPiAgICogVGhpcyBpbmZvcm1zIGNmZzgwMjExIHRoYXQg
QlNTIGluZm9ybWF0aW9uIHdhcyBmb3VuZCBhbmQNCj4gICAqIHRoZSBCU1Mgc2hvdWxkIGJlIHVw
ZGF0ZWQvYWRkZWQuDQo+ICsgKg0KPiArICogTk9URTogUmV0dXJucyBhIHJlZmVyZW5jZWQgc3Ry
dWN0LCBtdXN0IGJlIHJlbGVhc2VkIHdpdGggY2ZnODAyMTFfcHV0X2JzcygpIQ0KPiAgICovDQo+
IC1zdHJ1Y3QgY2ZnODAyMTFfYnNzKg0KPiArc3RydWN0IGNmZzgwMjExX2JzcyAqIF9fbXVzdF9j
aGVjaw0KPiAgY2ZnODAyMTFfaW5mb3JtX2JzcyhzdHJ1Y3Qgd2lwaHkgKndpcGh5LA0KPiAgCQkg
ICAgc3RydWN0IGllZWU4MDIxMV9jaGFubmVsICpjaGFubmVsLA0KPiAgCQkgICAgY29uc3QgdTgg
KmJzc2lkLA0KPiANCj4gDQo+IC0tDQo+IFRvIHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBz
ZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBsaW51eC13aXJlbGVzcyIgaW4NCj4gdGhlIGJvZHkg
b2YgYSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcNCj4gTW9yZSBtYWpvcmRv
bW8gaW5mbyBhdCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sDQo=

2011-10-28 10:08:51

by Dave Kilroy

[permalink] [raw]
Subject: [PATCH] orinoco: release BSS structures returned by cfg80211_inform_bss()

The pointer returned by cfg80211_inform_bss is a referenced
struct. The orinoco driver does not need to keep the struct, so
we just release it.

Signed-off-by: David Kilroy <[email protected]>
---
drivers/net/wireless/orinoco/scan.c | 18 ++++++++++++------
1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/orinoco/scan.c b/drivers/net/wireless/orinoco/scan.c
index e99ca1c..93674d9 100644
--- a/drivers/net/wireless/orinoco/scan.c
+++ b/drivers/net/wireless/orinoco/scan.c
@@ -76,6 +76,7 @@ static void orinoco_add_hostscan_result(struct orinoco_private *priv,
{
struct wiphy *wiphy = priv_to_wiphy(priv);
struct ieee80211_channel *channel;
+ struct cfg80211_bss *cbss;
u8 *ie;
u8 ie_buf[46];
u64 timestamp;
@@ -121,9 +122,11 @@ static void orinoco_add_hostscan_result(struct orinoco_private *priv,
beacon_interval = le16_to_cpu(bss->a.beacon_interv);
signal = SIGNAL_TO_MBM(le16_to_cpu(bss->a.level));

- cfg80211_inform_bss(wiphy, channel, bss->a.bssid, timestamp,
- capability, beacon_interval, ie_buf, ie_len,
- signal, GFP_KERNEL);
+ cbss = cfg80211_inform_bss(wiphy, channel, bss->a.bssid, timestamp,
+ capability, beacon_interval, ie_buf, ie_len,
+ signal, GFP_KERNEL);
+ if (cbss)
+ cfg80211_put_bss(cbss);
}

void orinoco_add_extscan_result(struct orinoco_private *priv,
@@ -132,6 +135,7 @@ void orinoco_add_extscan_result(struct orinoco_private *priv,
{
struct wiphy *wiphy = priv_to_wiphy(priv);
struct ieee80211_channel *channel;
+ struct cfg80211_bss *cbss;
const u8 *ie;
u64 timestamp;
s32 signal;
@@ -152,9 +156,11 @@ void orinoco_add_extscan_result(struct orinoco_private *priv,
ie = bss->data;
signal = SIGNAL_TO_MBM(bss->level);

- cfg80211_inform_bss(wiphy, channel, bss->bssid, timestamp,
- capability, beacon_interval, ie, ie_len,
- signal, GFP_KERNEL);
+ cbss = cfg80211_inform_bss(wiphy, channel, bss->bssid, timestamp,
+ capability, beacon_interval, ie, ie_len,
+ signal, GFP_KERNEL);
+ if (cbss)
+ cfg80211_put_bss(cbss);
}

void orinoco_add_hostscan_results(struct orinoco_private *priv,
--
1.7.4.1