iris_register_video_device() allocates a video_device with
video_device_alloc() and releases it from the err_vdev_release error path
if video_register_device() fails.

This can double free the video_device when __video_register_device()
reaches device_register() and that call fails:

  video_register_device()
    -> __video_register_device()
       -> device_register() fails
          -> put_device(&vdev->dev)
             -> v4l2_device_release()
                -> vdev->release(vdev)
                   -> video_device_release(vdev)

  iris_register_video_device()
    -> err_vdev_release
       -> video_device_release(vdev)

Use video_device_release_empty() while registering the device so that
registration failure paths do not free vdev through vdev->release().
iris_register_video_device() then releases vdev exactly once from
err_vdev_release. Restore video_device_release() after successful
registration so the registered device keeps its normal lifetime handling.

Clear the cached decoder or encoder video_device pointer on failure since
it is assigned before video_register_device().

This issue was found by a static analysis tool I am developing.

Fixes: 38506cb7e8d2 ("media: iris: add platform driver for iris video device")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/media/platform/qcom/iris/iris_probe.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
index ddaacda523ec..0d259f9b22a1 100644
--- a/drivers/media/platform/qcom/iris/iris_probe.c
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -151,7 +151,7 @@ static int iris_register_video_device(struct iris_core *core, enum domain_type t
 	if (!vdev)
 		return -ENOMEM;
 
-	vdev->release = video_device_release;
+	vdev->release = video_device_release_empty;
 	vdev->fops = core->iris_v4l2_file_ops;
 	vdev->vfl_dir = VFL_DIR_M2M;
 	vdev->v4l2_dev = &core->v4l2_dev;
@@ -174,11 +174,17 @@ static int iris_register_video_device(struct iris_core *core, enum domain_type t
 	if (ret)
 		goto err_vdev_release;
 
+	vdev->release = video_device_release;
 	video_set_drvdata(vdev, core);
 
 	return 0;
 
 err_vdev_release:
+	if (core->vdev_dec == vdev)
+		core->vdev_dec = NULL;
+	if (core->vdev_enc == vdev)
+		core->vdev_enc = NULL;
+
 	video_device_release(vdev);
 
 	return ret;
-- 
2.43.0

On Mon, May 18, 2026 at 06:57:55PM +0800, Guangshuo Li wrote:
> iris_register_video_device() allocates a video_device with
> video_device_alloc() and releases it from the err_vdev_release error path
> if video_register_device() fails.
> 
> This can double free the video_device when __video_register_device()
> reaches device_register() and that call fails:
> 
>   video_register_device()
>     -> __video_register_device()
>        -> device_register() fails
>           -> put_device(&vdev->dev)
>              -> v4l2_device_release()
>                 -> vdev->release(vdev)
>                    -> video_device_release(vdev)
> 
>   iris_register_video_device()
>     -> err_vdev_release
>        -> video_device_release(vdev)
> 
> Use video_device_release_empty() while registering the device so that
> registration failure paths do not free vdev through vdev->release().
> iris_register_video_device() then releases vdev exactly once from
> err_vdev_release. Restore video_device_release() after successful
> registration so the registered device keeps its normal lifetime handling.

This is definitely not the correct way to handle the issue. Fix the
error path instead.

> 
> Clear the cached decoder or encoder video_device pointer on failure since
> it is assigned before video_register_device().
> 
> This issue was found by a static analysis tool I am developing.
> 
> Fixes: 38506cb7e8d2 ("media: iris: add platform driver for iris video device")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/media/platform/qcom/iris/iris_probe.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 

-- 
With best wishes
Dmitry

Hi Dmitry,

Thanks for reviewing.

On Tue, 19 May 2026 at 01:12, Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On Mon, May 18, 2026 at 06:57:55PM +0800, Guangshuo Li wrote:
> > iris_register_video_device() allocates a video_device with
> > video_device_alloc() and releases it from the err_vdev_release error path
> > if video_register_device() fails.
> >
> > This can double free the video_device when __video_register_device()
> > reaches device_register() and that call fails:
> >
> >   video_register_device()
> >     -> __video_register_device()
> >        -> device_register() fails
> >           -> put_device(&vdev->dev)
> >              -> v4l2_device_release()
> >                 -> vdev->release(vdev)
> >                    -> video_device_release(vdev)
> >
> >   iris_register_video_device()
> >     -> err_vdev_release
> >        -> video_device_release(vdev)
> >
> > Use video_device_release_empty() while registering the device so that
> > registration failure paths do not free vdev through vdev->release().
> > iris_register_video_device() then releases vdev exactly once from
> > err_vdev_release. Restore video_device_release() after successful
> > registration so the registered device keeps its normal lifetime handling.
>
> This is definitely not the correct way to handle the issue. Fix the
> error path instead.
>

I had also considered fixing this by changing the error path, but I am
a bit concerned about the interaction with the device_register()
failure path in __video_register_device().

Commit 2a934fdb01db ("media: v4l2-dev: fix error handling in
__video_register_device()") added put_device() after device_register()
fails, because after calling device_register(), the device must be
released with put_device() even if device_register() returns an error.
Otherwise the reference initialized by the driver core is not dropped,
which can cause a memory leak.

On the other hand, if I simply remove video_device_release() from the
video_register_device() failure path in iris_register_video_device(),
then earlier failures in __video_register_device() would leak the
video_device. Those earlier failures happen before device_register()
is called, so put_device() is not used and vdev->release() is not
invoked. In that case, the video_device allocated by
video_device_alloc() is still owned by the caller and still needs to
be released by video_device_release().

So there seem to be two different failure cases:

before device_register(): caller still needs video_device_release()

device_register() failure: __video_register_device() calls
put_device(), which may already invoke vdev->release()

Would you please share any suggestions on how to fix this issue properly?

Best regards,
Guangshuo