Re: [RFC PATCH 02/11] media: iris: Add hooks to initialize and tear down context banks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The `iris_core` structure is allocated with `devm_kzalloc` but referenced by video devices that can outlive the device unbind process.
- [High] Missing cancellation of the `sys_error_handler` delayed work in `iris_remove`.
- [High] Hardware and queues are deinitialized before userspace access is revoked in `iris_remove`.
--

--- Patch [2]: [RFC PATCH 02/11] media: iris: Add hooks to initialize and tear down context banks ---
commit 746928806b2d799681dddbd3d539730ff7f9d31a
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>

media: iris: Add hooks to initialize and tear down context banks

This commit adds platform hooks to set up and tear down context bank
devices. It updates the platform data structure to include these hooks
and modifies the probe and remove functions to call them.

> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> index c2dcb50a27824..0bce4fb1786bc 100644
> --- a/drivers/media/platform/qcom/iris/iris_probe.c
> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
[ ... ]
> @@ -207,6 +221,8 @@ static void iris_remove(struct platform_device *pdev)
>  
>  	v4l2_device_unregister(&core->v4l2_dev);
>  
> +	iris_deinit_cb_devs(core);
> +

[Severity: High]
This is a pre-existing issue, but are the hardware and queues deinitialized
before userspace access is revoked?

iris_core_deinit(core) is called before video_unregister_device() (higher up
in this function). Because the video device is still registered, userspace
can concurrently call open().

iris_open() calls iris_core_init(), which sees IRIS_CORE_DEINIT and
re-allocates the DMA queues and powers on the hardware. When iris_remove()
completes, devres frees core, but the newly allocated DMA memory is leaked.

Furthermore, the successfully opened file descriptor will now operate on a
freed core structure, leading to a use-after-free.

[Severity: High]
This is a pre-existing issue, but should the sys_error_handler delayed work
be canceled here?

The delayed work sys_error_handler is scheduled by the HFI interrupt handler
when a system error occurs. Because the interrupt is devm-managed, it
remains active throughout iris_remove().

If the interrupt fires and schedules the work during unbind, the work will
execute asynchronously after iris_remove() completes and devres frees core.
The work handler iris_sys_error_handler() will then dereference the freed
core structure.

>  	mutex_destroy(&core->lock);
>  }
>  
> @@ -273,10 +289,14 @@ static int iris_probe(struct platform_device *pdev)
>  
>  	iris_session_init_caps(core);
>  
> -	ret = v4l2_device_register(dev, &core->v4l2_dev);
> +	ret = iris_init_cb_devs(core);
>  	if (ret)
>  		return ret;
>  
> +	ret = v4l2_device_register(dev, &core->v4l2_dev);
> +	if (ret)
> +		goto err_cb_deinit;
> +
>  	ret = iris_register_video_device(core, DECODER);

[Severity: High]
This is a pre-existing issue, but can the core structure outlive its
allocation?

The core structure is allocated with devm_kzalloc() earlier in probe, which
ties its lifetime to the device unbind process. However,
video_unregister_device() does not force-close active file descriptors.

If a user-space application holds the video node open while the driver is
unbound, devres will free the core structure. When the user application later
calls close(), V4L2 invokes iris_close():

drivers/media/platform/qcom/iris/iris_vidc.c:iris_close() {
    struct iris_inst *inst = iris_get_inst(filp);
    ...
    iris_remove_session(inst);
}

This path attempts to acquire core->lock on the already freed core object.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vpu_iommu_iova_handling-v1-0-72bb62cb2dfd@oss.qualcomm.com?part=2