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

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 <redacted>
---
 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

Stanse found a memory leak in cfg80211_wext_siwscan. creq is not
freed/assigned on all paths. Fix that.

Signed-off-by: Jiri Slaby <redacted>
---
 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);
 

-- 
1.6.5.7

On Wed, Jan 6, 2010 at 8:35 AM, Jiri Slaby [off-list ref] wrote:

Stanse found a memory leak in cfg80211_wext_siwscan. creq is not
freed/assigned on all paths. Fix that.

CC stable?

 Luis

On Wed, Jan 6, 2010 at 8:35 AM, Jiri Slaby [off-list ref] 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

On 01/06/10 17:35, Jiri Slaby wrote:

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.

On 01/06/2010 08:07 PM, Gertjan van Wingerde wrote:

On 01/06/10 17:35, Jiri Slaby wrote:

quoted

@@ -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

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 <redacted>

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

commit 65486c8b30498dd274eea2c542696f22b63fe5b8
Author: Johannes Berg [off-list ref]
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 [off-list ref]
    Signed-off-by: Johannes Berg [off-list ref]
    Acked-by: Dan Carpenter [off-list ref]
    Signed-off-by: John W. Linville [off-list ref]

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

On 01/06/2010 08:55 PM, John W. Linville wrote:

On Wed, Jan 06, 2010 at 05:35:42PM +0100, Jiri Slaby wrote:

quoted

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 <redacted>

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

Ok, thanks.
-- 
js