linux-kernel - RE: [PATCH v2] mwifiex: fix race condition when downloading firmware

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:	Thu, 9 Jun 2016 13:30:44 +0000
From:	Amitkumar Karwar <akarwar@...vell.com>
To:	Wei-Ning Huang <wnhuang@...omium.org>,
	Linux-Wireless <linux-wireless@...r.kernel.org>
CC:	LKML <linux-kernel@...r.kernel.org>,
	"djkurtz@...omium.org" <djkurtz@...omium.org>
Subject: RE: [PATCH v2] mwifiex: fix race condition when downloading firmware

> From: Wei-Ning Huang [mailto:wnhuang@...omium.org]
> Sent: Friday, May 27, 2016 5:58 PM
> To: Linux-Wireless
> Cc: LKML; Amitkumar Karwar; djkurtz@...omium.org; Wei-Ning Huang
> Subject: [PATCH v2] mwifiex: fix race condition when downloading
> firmware
> 
> The action 'check for winner' and 'download firmware' should be an
> atomic action. This is true for btmrvl driver but not mwmfiex, which
> cause firmware download to fail when the following scenario happens:
> 
> 1) mwifiex check winner status: true
> 2) btmrvl check winner status: true, and start downloading firmware
> 3) mwifiex tries to download firmware, but failed because btmrvl is
> already downloading.
> 
> This won't happen if 1) and 3) is an atomic action. This patch adds
> sdio_claim/release_host call around those two actions to make sure it's
> atomic.
> 
> Signed-off-by: Wei-Ning Huang <wnhuang@...omium.org>
> ---
>  drivers/net/wireless/marvell/mwifiex/init.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/init.c
> b/drivers/net/wireless/marvell/mwifiex/init.c
> index 78c532f..7703438 100644
> --- a/drivers/net/wireless/marvell/mwifiex/init.c
> +++ b/drivers/net/wireless/marvell/mwifiex/init.c
> @@ -24,6 +24,7 @@
>  #include "main.h"
>  #include "wmm.h"
>  #include "11n.h"
> +#include "sdio.h"
> 
>  /*
>   * This function adds a BSS priority table to the table list.
> @@ -737,16 +738,20 @@ mwifiex_shutdown_drv(struct mwifiex_adapter
> *adapter)  int mwifiex_dnld_fw(struct mwifiex_adapter *adapter,
>  		    struct mwifiex_fw_image *pmfw)
>  {
> +	struct sdio_mmc_card *card = adapter->card;
>  	int ret;
>  	u32 poll_num = 1;
> 
> +	if (adapter->iface_type == MWIFIEX_SDIO)
> +		sdio_claim_host(card->func);
> +
>  	if (adapter->if_ops.check_fw_status) {
>  		/* check if firmware is already running */
>  		ret = adapter->if_ops.check_fw_status(adapter, poll_num);
>  		if (!ret) {
>  			mwifiex_dbg(adapter, MSG,
>  				    "WLAN FW already running! Skip FW dnld\n");
> -			return 0;
> +			goto release_host;
>  		}
>  	}
> 
> @@ -759,7 +764,7 @@ int mwifiex_dnld_fw(struct mwifiex_adapter *adapter,
>  		if (ret) {
>  			mwifiex_dbg(adapter, MSG,
>  				    "WLAN read winner status failed!\n");
> -			return ret;
> +			goto release_host;
>  		}
> 
>  		if (!adapter->winner) {
> @@ -775,7 +780,7 @@ int mwifiex_dnld_fw(struct mwifiex_adapter *adapter,
>  		if (ret) {
>  			mwifiex_dbg(adapter, ERROR,
>  				    "prog_fw failed ret=%#x\n", ret);
> -			return ret;
> +			goto release_host;
>  		}
>  	}
> 
> @@ -785,6 +790,9 @@ poll_fw:
>  	if (ret)
>  		mwifiex_dbg(adapter, ERROR,
>  			    "FW failed to be active in time\n");
> +release_host:
> +	if (adapter->iface_type == MWIFIEX_SDIO)
> +		sdio_release_host(card->func);
> 
>  	return ret;
>  }
> --
> 2.1.2

Patch looks fine to me.

Acked-by: Amitkumar Karwar <akarwar@...vell.com>

Regards,
Amitkumar