linux-kernel - Re: [PATCH] wireless: wl12xx, fix lock imbalance

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 14 Jul 2009 08:44:02 +0300
From:	Luciano Coelho <luciano.coelho@...ia.com>
To:	ext Jiri Slaby <jirislaby@...il.com>
CC:	"linville@...driver.com" <linville@...driver.com>,
	"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] wireless: wl12xx, fix lock imbalance

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

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