Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH 1/2] net: wireless, fix lock imbalance

3 views
Skip to first unread message

Jiri Slaby

unread,
Jan 6, 2010, 11:40:01 AM1/6/10
to
One fail path in cfg80211_wext_siwscan omits to unlock rdev->mtx.
Fix that.

Trigerrable by "Scan for SSID" with long enough SSID (> 32).

Signed-off-by: Jiri Slaby <jsl...@suse.cz>
---
net/wireless/scan.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 12dfa62..9c50c85 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -694,8 +694,10 @@ int cfg80211_wext_siwscan(struct net_device *dev,
/* translate "Scan for SSID" request */
if (wreq) {
if (wrqu->data.flags & IW_SCAN_THIS_ESSID) {
- if (wreq->essid_len > IEEE80211_MAX_SSID_LEN)
- return -EINVAL;
+ if (wreq->essid_len > IEEE80211_MAX_SSID_LEN) {
+ err = -EINVAL;
+ goto out;
+ }
memcpy(creq->ssids[0].ssid, wreq->essid, wreq->essid_len);
creq->ssids[0].ssid_len = wreq->essid_len;
}
--
1.6.5.7

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Jiri Slaby

unread,
Jan 6, 2010, 11:40:02 AM1/6/10
to
Stanse found a memory leak in cfg80211_wext_siwscan. creq is not
freed/assigned on all paths. Fix that.

Signed-off-by: Jiri Slaby <jsl...@suse.cz>
---

net/wireless/scan.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 9c50c85..3003d13 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -685,7 +685,7 @@ int cfg80211_wext_siwscan(struct net_device *dev,
/* No channels found? */
if (!i) {
err = -EINVAL;
- goto out;
+ goto out_free;
}

/* Set real number of channels specified in creq->channels[] */
@@ -696,7 +696,7 @@ int cfg80211_wext_siwscan(struct net_device *dev,


if (wrqu->data.flags & IW_SCAN_THIS_ESSID) {

if (wreq->essid_len > IEEE80211_MAX_SSID_LEN) {
err = -EINVAL;
- goto out;
+ goto out_free;


}
memcpy(creq->ssids[0].ssid, wreq->essid, wreq->essid_len);
creq->ssids[0].ssid_len = wreq->essid_len;

@@ -717,6 +717,9 @@ int cfg80211_wext_siwscan(struct net_device *dev,
out:
cfg80211_unlock_rdev(rdev);
return err;
+out_free:
+ kfree(creq);
+ goto out;
}
EXPORT_SYMBOL_GPL(cfg80211_wext_siwscan);

Luis R. Rodriguez

unread,
Jan 6, 2010, 12:50:01 PM1/6/10
to
On Wed, Jan 6, 2010 at 8:35 AM, Jiri Slaby <jsl...@suse.cz> wrote:
> One fail path in cfg80211_wext_siwscan omits to unlock rdev->mtx.
> Fix that.
>
> Trigerrable by "Scan for SSID" with long enough SSID (> 32).

Cc stable as well?

Luis

Luis R. Rodriguez

unread,
Jan 6, 2010, 12:50:02 PM1/6/10
to
On Wed, Jan 6, 2010 at 8:35 AM, Jiri Slaby <jsl...@suse.cz> wrote:
> Stanse found a memory leak in cfg80211_wext_siwscan. creq is not
> freed/assigned on all paths. Fix that.

CC stable?

Luis

Gertjan van Wingerde

unread,
Jan 6, 2010, 2:10:03 PM1/6/10
to

This last part looks a bit strange. Why don't you put out_free label before out label,
and let it continue after the kfree, instead of jumping back to the out label.

---
Gertjan.

Jiri Slaby

unread,
Jan 6, 2010, 2:20:02 PM1/6/10
to
On 01/06/2010 08:07 PM, Gertjan van Wingerde wrote:
> On 01/06/10 17:35, Jiri Slaby wrote:
>> @@ -717,6 +717,9 @@ int cfg80211_wext_siwscan(struct net_device *dev,
>> out:
>> cfg80211_unlock_rdev(rdev);
>> return err;
>> +out_free:
>> + kfree(creq);
>> + goto out;
>> }
>> EXPORT_SYMBOL_GPL(cfg80211_wext_siwscan);
>>
>
> This last part looks a bit strange. Why don't you put out_free label before out label,

Because `out' is a part of non-error flow, where the creq should not be
freed.

thanks,
--
js

John W. Linville

unread,
Jan 6, 2010, 3:10:02 PM1/6/10
to
On Wed, Jan 06, 2010 at 05:35:42PM +0100, Jiri Slaby wrote:
> One fail path in cfg80211_wext_siwscan omits to unlock rdev->mtx.
> Fix that.
>
> Trigerrable by "Scan for SSID" with long enough SSID (> 32).
>
> Signed-off-by: Jiri Slaby <jsl...@suse.cz>

Both this and the next one are already fixed in wireless-2.6...

commit 65486c8b30498dd274eea2c542696f22b63fe5b8
Author: Johannes Berg <joha...@sipsolutions.net>
Date: Wed Dec 23 15:33:35 2009 +0100

cfg80211: fix error path in cfg80211_wext_siwscan

If there's an invalid channel or SSID, the code leaks
the scan request. Always free the scan request, unless
it was successfully given to the driver.

Reported-by: Dan Carpenter <err...@gmail.com>
Signed-off-by: Johannes Berg <joha...@sipsolutions.net>
Acked-by: Dan Carpenter <err...@gmail.com>
Signed-off-by: John W. Linville <linv...@tuxdriver.com>

--
John W. Linville Someday the world will need a hero, and you
linv...@tuxdriver.com might be all we have. Be ready.

Jiri Slaby

unread,
Jan 6, 2010, 3:40:01 PM1/6/10
to
On 01/06/2010 08:55 PM, John W. Linville wrote:
> On Wed, Jan 06, 2010 at 05:35:42PM +0100, Jiri Slaby wrote:
>> One fail path in cfg80211_wext_siwscan omits to unlock rdev->mtx.
>> Fix that.
>>
>> Trigerrable by "Scan for SSID" with long enough SSID (> 32).
>>
>> Signed-off-by: Jiri Slaby <jsl...@suse.cz>
>
> Both this and the next one are already fixed in wireless-2.6...

Ok, thanks.
--
js

0 new messages