On Tue, Dec 17, 2024 at 05:32:20PM +0300, Dan Carpenter wrote:
> On Sat, Dec 14, 2024 at 05:48:53PM +0900, Joe Hattori wrote:
> > When the device_register() in __video_register_device() fails, current
> > implementation does not decrement the refcount of the device which was
> > obtained in device_initialize(). Balance the refcount by calling
> > put_device() before jumping to the cleanup label.
> >
> > This bug was found by an experimental static analysis tool that I am
> > developing.
> >
> > Fixes: 5bc3cb743bba ("[media] v4l: move v4l2 core into a separate directory")
> > Signed-off-by: Joe Hattori <joe@xxxxxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/media/v4l2-core/v4l2-dev.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> > index 5bcaeeba4d09..1e48778cd7de 100644
> > --- a/drivers/media/v4l2-core/v4l2-dev.c
> > +++ b/drivers/media/v4l2-core/v4l2-dev.c
> > @@ -1058,6 +1058,7 @@ int __video_register_device(struct video_device *vdev,
> > mutex_lock(&videodev_lock);
> > ret = device_register(&vdev->dev);
> > if (ret < 0) {
> > + put_device(&vdev->dev);
>
> You're very brave. ;) I've looked at this before and concluded that it
> was better to leak.
>
> For example, when this is called from zoran_init_video_device() via
> video_register_device() then the release function is zoran_vdev_release()
> which will free vdev. So the goto cleanup will have use after frees.
>
> I don't think there is a way to fix some of this.
I guess the way that vdev is supposed to be freed is that if
video_register_device() fails, then you're supposed to call
video_device_release(). It leaks some kobj debug code probably but
that's not life threatening.
regards,
dan carpenter
