linux-kernel - Re: [PATCH 1/2] media: tuner xc5000 - release firmwware from xc5000

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:	Mon, 22 Sep 2014 15:32:17 -0600
From:	Shuah Khan <shuahkh@....samsung.com>
To:	Mauro Carvalho Chehab <mchehab@....samsung.com>,
	Shuah Khan <shuah.kh@...sung.com>
CC:	fabf@...net.be, linux-media@...r.kernel.org,
	linux-kernel@...r.kernel.org, Shuah Khan <shuahkh@....samsung.com>
Subject: Re: [PATCH 1/2] media: tuner xc5000 - release firmwware from xc5000_release()

On 09/22/2014 02:18 PM, Mauro Carvalho Chehab wrote:
> Em Wed, 13 Aug 2014 19:09:23 -0600
> Shuah Khan <shuah.kh@...sung.com> escreveu:
> 
>> xc5000 releases firmware right after loading it. Change it to
>> save the firmware and release it from xc5000_release(). This
>> helps avoid fecthing firmware when forced firmware load requests
>> come in to change analog tv frequence and when firmware needs to
>> be reloaded after suspend and resume.
>>
>> Signed-off-by: Shuah Khan <shuah.kh@...sung.com>
>> ---
>>  drivers/media/tuners/xc5000.c |   34 ++++++++++++++++++++--------------
>>  1 file changed, 20 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/media/tuners/xc5000.c b/drivers/media/tuners/xc5000.c
>> index 512fe50..31b1dec 100644
>> --- a/drivers/media/tuners/xc5000.c
>> +++ b/drivers/media/tuners/xc5000.c
>> @@ -70,6 +70,8 @@ struct xc5000_priv {
>>  
>>  	struct dvb_frontend *fe;
>>  	struct delayed_work timer_sleep;
>> +
>> +	const struct firmware   *firmware;
>>  };
>>  
>>  /* Misc Defines */
>> @@ -1136,20 +1138,23 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe, int force)
>>  	if (!force && xc5000_is_firmware_loaded(fe) == 0)
>>  		return 0;
>>  
>> -	ret = request_firmware(&fw, desired_fw->name,
>> -			       priv->i2c_props.adap->dev.parent);
>> -	if (ret) {
>> -		printk(KERN_ERR "xc5000: Upload failed. (file not found?)\n");
>> -		return ret;
>> -	}
>> -
>> -	dprintk(1, "firmware read %Zu bytes.\n", fw->size);
>> +	if (!priv->firmware) {
>> +		ret = request_firmware(&fw, desired_fw->name,
>> +					priv->i2c_props.adap->dev.parent);
>> +		if (ret) {
>> +			pr_err("xc5000: Upload failed. rc %d\n", ret);
>> +			return ret;
>> +		}
>> +		dprintk(1, "firmware read %Zu bytes.\n", fw->size);
>>  
>> -	if (fw->size != desired_fw->size) {
>> -		printk(KERN_ERR "xc5000: Firmware file with incorrect size\n");
>> -		ret = -EINVAL;
>> -		goto err;
>> -	}
>> +		if (fw->size != desired_fw->size) {
>> +			pr_err("xc5000: Firmware file with incorrect size\n");
>> +			ret = -EINVAL;
>> +			goto err;
> 
> In this case, we should be releasing the firmware too. Btw, this code will
> also add a memory leak, as priv->firmware will be null, but the firmware
> was loaded.

Yes you are right. I will revise the patch series and resend it.

-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh@....samsung.com | (970) 217-8978
--
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/