Re: [PATCH] media: v4l2-dev: do not fire driver's release on __video_register_device() failure

[Guangshuo Li]

Hi Sakari,

Thanks for reviewing.

On Wed, 20 May 2026 at 18:01, Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
>
> Hi Guangshuo,
>
> Thanks for the patch.
>
> On Wed, May 20, 2026 at 05:06:24PM +0800, Guangshuo Li wrote:
> > video_register_device() / __video_register_device() registers vdev->dev
> > with device_register(). Before the call the video core sets
> >
> >       vdev->dev.release = v4l2_device_release;
> >
> > v4l2_device_release() invokes vdev->release(vdev) as its last step, and
> > the driver's vdev->release hook is commonly video_device_release(), which
> > kfree()s the vdev that the driver allocated with video_device_alloc().
> >
> > When device_register() fails inside __video_register_device() the core
> > does
> >
> >       put_device(&vdev->dev);
> >       return ret;
> >
> > which drops the only reference and fires the v4l2_device_release()
> > chain:
> >
> >   __video_register_device()
> >     device_register() -> -E*
> >     put_device(&vdev->dev)
> >       -> v4l2_device_release()
> >          -> vdev->release(vdev)
> >             -> video_device_release(vdev)   /* kfree(vdev), free #1 */
> >
> > video_register_device() returns the error to the driver. Drivers that
> > follow the documented ownership contract release vdev on their own error
> > path, e.g.
> >
> >   driver_probe()
> >     if (video_register_device(vdev, ...))
> >       goto err_release_vdev;
> >     ...
> >   err_release_vdev:
> >     video_device_release(vdev);   /* free #2 -- DOUBLE FREE */
> >
> > This is the contract documented in
> > Documentation/driver-api/media/v4l2-dev.rst: the driver owns vdev and
> > is responsible for releasing it if video_register_device() fails. As
> > Hans Verkuil pointed out, the right place to fix this is the v4l2 core
> > rather than every individual driver, because drivers are expected to
> > follow the documented ownership contract.
> >
> > Neutralise vdev->release around put_device() in the device_register()
> > failure path so the device core cleanup does not run the driver's
> > release hook. The driver-supplied release is restored before returning
> > so the caller can release vdev according to the documented contract.
> > Successful registration is unchanged, so the normal teardown sequence
> > continues to call the driver's release hook and free vdev exactly once on
> > unregister.
>
> May I ask how the issue was found?
>

The issue was found by a static analysis tool that I am currently developing.

The tool reported a few double-free issues around video_device
lifetime handling, especially in error paths after
video_register_device() failures. I first prepared patches for the
individual drivers where the pattern was reported.

After discussing this with Hans and others, we concluded that the
problem is better fixed in the V4L2 core, since drivers are following
the documented ownership model and the problematic case comes from the
device_register() failure path in __video_register_device().

Best regards,
Guangshuo