Re: [patch review] radio-si470x: fix possible bug with freeing memory order

2009-04-01 Thread Alexey Klimov
(add Mauro on c/c)

On Wed, Apr 1, 2009 at 9:36 PM, Tobias Lorenz  wrote:
> Hi,

Hi :)

> hey thanks. I appreciate and approve the patch.
>
> Do I have to upload the patch and send Mauro a pull request or is this
> already done?
>
> Bye,
>
> Toby

Well, as i understand that's to you or Mauro to decide.


> On Wednesday 01 April 2009 02:01:04 Alexey Klimov wrote:
>
>> Hello, all
>
>>
>
>> There is probably bug when cleanup occurs in si470x_usb_driver_probe.
>
>> We do kmalloc for radio->buffer and when it's fail we
>
>> kfree(radio->buffer). The same with si470x_get_all_registers() and
>
>> si470x_get_scratch_page_versions(). When this functions failed we go to
>
>> err_all and try to free radio->buffer before allocation memory for this.
>
>>
>
>> --
>
>> Patch fixes cleanup procedure in si470x_usb_driver_probe. Add new label
>
>> err_video and change order of freeing memory.
>
>>
>
>> Signed-off-by: Alexey Klimov 
>
>> --
>
>> diff -r 5567e82c34a0 linux/drivers/media/radio/radio-si470x.c
>
>> --- a/linux/drivers/media/radio/radio-si470x.c Tue Mar 31 07:24:14 2009
>> -0300
>
>> +++ b/linux/drivers/media/radio/radio-si470x.c Wed Apr 01 03:48:31 2009
>> +0400
>
>> @@ -1687,7 +1687,7 @@
>
>> /* show some infos about the specific si470x device */
>
>> if (si470x_get_all_registers(radio) < 0) {
>
>> retval = -EIO;
>
>> - goto err_all;
>
>> + goto err_video;
>
>> }
>
>> printk(KERN_INFO DRIVER_NAME ": DeviceID=0x%4.4hx ChipID=0x%4.4hx\n",
>
>> radio->registers[DEVICEID], radio->registers[CHIPID]);
>
>> @@ -1695,7 +1695,7 @@
>
>> /* get software and hardware versions */
>
>> if (si470x_get_scratch_page_versions(radio) < 0) {
>
>> retval = -EIO;
>
>> - goto err_all;
>
>> + goto err_video;
>
>> }
>
>> printk(KERN_INFO DRIVER_NAME
>
>> ": software version %d, hardware version %d\n",
>
>> @@ -1728,7 +1728,7 @@
>
>> radio->buffer = kmalloc(radio->buf_size, GFP_KERNEL);
>
>> if (!radio->buffer) {
>
>> retval = -EIO;
>
>> - goto err_all;
>
>> + goto err_video;
>
>> }
>
>>
>
>> /* rds buffer configuration */
>
>> @@ -1750,8 +1750,9 @@
>
>>
>
>> return 0;
>
>> err_all:
>
>> + kfree(radio->buffer);
>
>> +err_video:
>
>> video_device_release(radio->videodev);
>
>> - kfree(radio->buffer);
>
>> err_radio:
>
>> kfree(radio);
>
>> err_initial:
>
>>
>
>>
>
>>



-- 
Best regards, Klimov Alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[patch review] radio-si470x: fix possible bug with freeing memory order

2009-03-31 Thread Alexey Klimov
Hello, all

There is probably bug when cleanup occurs in si470x_usb_driver_probe.
We do kmalloc for radio->buffer and when it's fail we
kfree(radio->buffer). The same with si470x_get_all_registers() and 
si470x_get_scratch_page_versions(). When this functions failed we go to
err_all and try to free radio->buffer before allocation memory for this.

--
Patch fixes cleanup procedure in si470x_usb_driver_probe. Add new label
err_video and change order of freeing memory.

Signed-off-by: Alexey Klimov 
--
diff -r 5567e82c34a0 linux/drivers/media/radio/radio-si470x.c
--- a/linux/drivers/media/radio/radio-si470x.c  Tue Mar 31 07:24:14 2009 -0300
+++ b/linux/drivers/media/radio/radio-si470x.c  Wed Apr 01 03:48:31 2009 +0400
@@ -1687,7 +1687,7 @@
/* show some infos about the specific si470x device */
if (si470x_get_all_registers(radio) < 0) {
retval = -EIO;
-   goto err_all;
+   goto err_video;
}
printk(KERN_INFO DRIVER_NAME ": DeviceID=0x%4.4hx ChipID=0x%4.4hx\n",
radio->registers[DEVICEID], radio->registers[CHIPID]);
@@ -1695,7 +1695,7 @@
/* get software and hardware versions */
if (si470x_get_scratch_page_versions(radio) < 0) {
retval = -EIO;
-   goto err_all;
+   goto err_video;
}
printk(KERN_INFO DRIVER_NAME
": software version %d, hardware version %d\n",
@@ -1728,7 +1728,7 @@
radio->buffer = kmalloc(radio->buf_size, GFP_KERNEL);
if (!radio->buffer) {
retval = -EIO;
-   goto err_all;
+   goto err_video;
}
 
/* rds buffer configuration */
@@ -1750,8 +1750,9 @@
 
return 0;
 err_all:
+   kfree(radio->buffer);
+err_video:
video_device_release(radio->videodev);
-   kfree(radio->buffer);
 err_radio:
kfree(radio);
 err_initial:



-- 
Best regards, Klimov Alexey

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html