Hi,

I've been looking deeper into the em28xx and em28xx-dvb modules, and I'm concerned that there are some races and resource leaks inherent in the current code:

a) Shouldn't em28xx_init_extension() and em28xx_add_into_devlist() be unified into a single function? Otherwise, consider someone plugging a DVB adapter into a host when the em28xx-dvb module is not yet loaded:

- em28xx_init_dev() adds new device to list.
- em28xx-dvb module registers itself, and initialises every device in the list (including our new one). - em28xx_init_dev() iterates over the list of extensions (including em28xx-dvb) with the new device.

At this point, dvb_init() has been called twice for our new device, resulting in a leaked struct em28xx_dvb.

b) When em28xx_init_dev() returns something != 0, em28xx_usb_probe() frees the struct em28xx and exits without calling usb_put_dev().

c) There are many ways that em28xx_init_dev() can return something != 0, and not all of them release the V4L2 device or I2C device.

Am I understanding this code correctly, please? I can obviously extend my patch accordingly - it is currently running without any obvious problems, but I only have one DVB adapter and none that uses the ALSA extension.

Cheers,
Chris

--
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

Reply via email to