[PATCH] wireless: wl12xx, fix lock imbalance

[PATCH] wireless: wl12xx, fix lock imbalance

[Jiri Slaby]

Add omitted mutex_unlock to one of wl12xx_op_start fail paths (when
wl12xx_chip_wakeup fails).

Not sure if the device should be powered off?

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
---
 drivers/net/wireless/wl12xx/main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 603d611..d241e4a 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -336,7 +336,7 @@ static int wl12xx_op_start(struct ieee80211_hw *hw)
 
 	ret = wl12xx_chip_wakeup(wl);
 	if (ret < 0)
-		return ret;
+		goto unlock;
 
 	ret = wl->chip.op_boot(wl);
 	if (ret < 0)
@@ -357,7 +357,7 @@ static int wl12xx_op_start(struct ieee80211_hw *hw)
 out:
 	if (ret < 0)
 		wl12xx_power_off(wl);
-
+unlock:
 	mutex_unlock(&wl->mutex);
 
 	return ret;
-- 
1.6.3.2

Re: [PATCH] wireless: wl12xx, fix lock imbalance

[Luciano Coelho]

ext Jiri Slaby wrote:
> Add omitted mutex_unlock to one of wl12xx_op_start fail paths (when
> wl12xx_chip_wakeup fails).
>   

Cool, very nice catch.  We actually just fixed this bug in our wl1271 
code (which I will hopefully send upstream this week), but we hadn't 
fixed it in the wl1251-specific code yet.

> Not sure if the device should be powered off?
>   

You should.  If the chip cannot be booted, why should it remain powered 
on? In some rare cases, the chip might fail to initialize, but can 
recover if powered off and on again, so turning it off at this point is 
the right thing to do.

> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
> ---
>   
>  drivers/net/wireless/wl12xx/main.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
> index 603d611..d241e4a 100644
> --- a/drivers/net/wireless/wl12xx/main.c
> +++ b/drivers/net/wireless/wl12xx/main.c
>   
> @@ -336,7 +336,7 @@ static int wl12xx_op_start(struct ieee80211_hw *hw)
>  
>  	ret = wl12xx_chip_wakeup(wl);
>  	if (ret < 0)
> -		return ret;
> +		goto unlock;
>   

Here you can just "goto out;" so that the chip is powered off before we 
return.

>  
>  	ret = wl->chip.op_boot(wl);
>  	if (ret < 0)
> @@ -357,7 +357,7 @@ static int wl12xx_op_start(struct ieee80211_hw *hw)
>  out:
>  	if (ret < 0)
>  		wl12xx_power_off(wl);
> -
> +unlock:
>  	mutex_unlock(&wl->mutex);
>  
>  	return ret;
>   

Thanks a lot for your patch!

-- 
Cheers,
Luca.