Re: [PATCH] [media] dib0700: Fix memory leak during initialization

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jean,

Em 12-02-2012 08:19, Jean Delvare escreveu:
> Reported by kmemleak.
> 
> Signed-off-by: Jean Delvare <khali@xxxxxxxxxxxx>
> Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxx>
> Cc: Devin Heitmueller <dheitmueller@xxxxxxxxxxxxxx>
> ---
> I am not familiar with the usb API, are we also supposed to call
> usb_kill_urb() in the error case maybe?
> 
>  drivers/media/dvb/dvb-usb/dib0700_core.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> --- linux-3.3-rc3.orig/drivers/media/dvb/dvb-usb/dib0700_core.c	2012-01-20 14:06:38.000000000 +0100
> +++ linux-3.3-rc3/drivers/media/dvb/dvb-usb/dib0700_core.c	2012-02-12 00:32:19.005334036 +0100
> @@ -787,6 +787,8 @@ int dib0700_rc_setup(struct dvb_usb_devi
>  	if (ret)
>  		err("rc submit urb failed\n");
>  
> +	usb_free_urb(purb);
> +
>  	return ret;
>  }
>  
> 

This patch doesn't sound right on my eyes, as you're freeing
an URB that you've just submitted _before_ having it handled
by the dib0700_rc_urb_completion() callback.

Btw, it seems that there's a bug at the fist if there:

static void dib0700_rc_urb_completion(struct urb *purb)
{
	struct dvb_usb_device *d = purb->context;
	struct dib0700_rc_response *poll_reply;
	u32 uninitialized_var(keycode);
	u8 toggle;

	deb_info("%s()\n", __func__);
	if (d == NULL)
		return;

	if (d->rc_dev == NULL) {
		/* This will occur if disable_rc_polling=1 */
		usb_free_urb(purb);
		return;
	}

...

it should be, instead:

	if (!d || !d->rc_dev) {
		/* This will occur if disable_rc_polling=1 */
		usb_free_urb(purb);
		return;
	}	


That's said, clearly there's no condition to stop the DVB IR
handling.

Probably, the right thing to do there is to add a function like:

int dib0700_disconnect(...) 
{
	usb_unlink_urb(urb);
	usb_free_urb(urb);

	dvb_usb_device_exit(...);
}

and use such function for the usb_driver disconnect handling: 

static struct usb_driver dib0700_driver = {
	.name       = "dvb_usb_dib0700",
	.probe      = dib0700_probe,
	.disconnect = dib0700_disconnect,
	.id_table   = dib0700_usb_id_table,
};

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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux