Hi Hans, On Mon, Oct 01, 2018 at 11:40:00AM +0200, Hans Verkuil wrote: > On 09/23/2018 06:34 PM, Akinobu Mita wrote: > > The video_i2c_data is allocated by kzalloc and released by the video > > device's release callback. The release callback is called when > > video_unregister_device() is called, but it will still be accessed after > > calling video_unregister_device(). > > > > Fix the use after free by allocating video_i2c_data by devm_kzalloc() with > > i2c_client->dev so that it will automatically be released when the i2c > > driver is removed. > > Hmm. The patch is right, but the explanation isn't. The core problem is > that vdev.release was set to video_i2c_release, but that should only be > used if struct video_device was kzalloc'ed. But in this case it is embedded > in a larger struct, and then vdev.release should always be set to > video_device_release_empty. When the driver is unbound, what's acquired using the devm_() family of functions is released. At the same time, the user still holds a file handle, and can issue IOCTLs --- but the device's data structures no longer exist. That's not ok, and also the reason why we have the release callback. While there are issues elsewhere, this bit of the V4L2 / MC frameworks is fine. Or am I missing something? -- Regards, Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx
