linux-kernel - Re: [PATCH 4/3] rt2860sta: call release_firmware on module exit, else resume is broken

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Sat, 18 Apr 2009 17:54:48 +0100
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Darren Salt <linux@...mustbejoking.demon.co.uk>
Cc:	Greg Kroah-Hartman <gregkh@...e.de>, linux-kernel@...r.kernel.org,
	David Woodhouse <dwmw2@...radead.org>
Subject: Re: [PATCH 4/3] rt2860sta: call release_firmware on module exit,
	else resume is broken

On Sat, 2009-04-18 at 16:15 +0100, Darren Salt wrote:
> Resume breakage is as follows: it takes somewhat longer (firmware request
> timeout), as a result of which the firmware isn't reloaded; and any
> subsequent attempt to trying to bring down the interface results in a panic &
> lockup (SysRq failure on my 901).
> 
> Consequently, it's better to request the firmware once and hang onto it until
> the module is unloaded.

Greg hasn't accepted the previous patch, so there's not much point
submitting patches on top of it yet.  I suggest you post a combined
patch once David has done a release of linux-firmware including
rt2860.bin.

> Signed-off-by: Darren Salt <linux@...mustbejoking.demon.co.uk>
> 
> diff -pur a/drivers/staging/rt2860/2860_main_dev.c b/drivers/staging/rt2860/2860_main_dev.c
> --- a/drivers/staging/rt2860/2860_main_dev.c
> +++ b/drivers/staging/rt2860/2860_main_dev.c
> @@ -289,6 +289,7 @@ static INT __init rt2860_init_module(VOI
>  static VOID __exit rt2860_cleanup_module(VOID)
>  {
>      pci_unregister_driver(&rt2860_driver);
> +    NICForgetFirmware();
>  }
>  
>  module_init(rt2860_init_module);
> diff -pur a/drivers/staging/rt2860/common/rtmp_init.c b/drivers/staging/rt2860/common/rtmp_init.c
> --- a/drivers/staging/rt2860/common/rtmp_init.c
> +++ b/drivers/staging/rt2860/common/rtmp_init.c
> @@ -144,6 +144,8 @@ RTMP_REG_PAIR	STAMACRegTable[] =	{
>  #define FW_FILENAME "rt2860.bin"
>  MODULE_FIRMWARE(FW_FILENAME);
>  
> +const struct firmware *fw = NULL;
> +

Must be static.

>  #define FIRMWAREIMAGE_MAX_LENGTH	0x2000
>  #define FIRMWARE_MAJOR_VERSION	0
>  #define FIRMWARE_MINOR_VERSION	2
> @@ -2583,6 +2585,12 @@ VOID NICEraseFirmware(
>  
>  }/* End of NICEraseFirmware */
>  
> +VOID NICForgetFirmware(void)
> +{
> +	release_firmware(fw);
> +	fw = NULL;
> +}
> +
>  /*
>  	========================================================================
>  
> @@ -2603,11 +2611,11 @@ VOID NICEraseFirmware(
>  NDIS_STATUS NICLoadFirmware(
>  	IN PRTMP_ADAPTER pAd)
>  {
> -	const struct firmware *fw;
>  	int ret, i;
>  	NDIS_STATUS Status = NDIS_STATUS_SUCCESS;
>  
> -	ret = request_firmware(&fw, FW_FILENAME,
> +	ret = fw ? 0 :
> +	      request_firmware(&fw, FW_FILENAME,
>  			       &((POS_COOKIE)pAd->OS_Cookie)->pci_dev->dev);

This is not thread-safe unless NICLoadFirmware is guarded by a global
lock.  Is it always called under RTNL?  If so, we can add ASSERT_RTNL()
to ensure that this doesn't change.

>  	if (ret) {
>  		printk(KERN_ERR "rt2860sta: firmware file %s request failed (%d)\n",
> @@ -2659,7 +2667,7 @@ NDIS_STATUS NICLoadFirmware(
>  	RTMP_IO_WRITE32(pAd, H2M_BBP_AGENT, 0);
>  	RTMP_IO_WRITE32(pAd, H2M_MAILBOX_CSR, 0);
>  
> -	release_firmware(fw);
> +	/*release_firmware(fw);*/
[...]

Just remove it.
 
Ben.


Download attachment "signature.asc" of type "application/pgp-signature" (190 bytes)