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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing cancellation of the delayed work `sys_error_handler` during driver removal, leading to a Use-After-Free if the timer fires after `iris_remove` completes.
- [High] Devres lifetime mismatch with video_device causing Use-After-Free on file operations if V4L2 file descriptors are kept open after driver unbind.
--

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

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

Add platform hooks to set up and tear down context bank devices.
Different iris platforms may require different context bank setup, so
let platform data provide the required operations via vpu_ops.

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

[Severity: High]
This is a pre-existing issue, but does this function need to cancel the
delayed work core->sys_error_handler before returning?

If the device triggers a hardware error, core->sys_error_handler is scheduled
with a delay. If the device is concurrently unbound (e.g., via sysfs) before
the timer expires, iris_remove() and iris_core_deinit() execute but fail to
cancel the delayed work.

Since the core structure is allocated using devm_kzalloc(), it is freed
automatically after iris_remove() returns. When the delayed work timer
eventually fires, iris_sys_error_handler() will dereference the freed core
pointer.

[Severity: High]
This is also a pre-existing issue, but is there a devres lifetime mismatch
with the video_device?

The core structure is allocated using devm_kzalloc() during probe, binding
its lifetime to the platform device bind/unbind lifecycle. However, userspace
can open the V4L2 character device and hold the file descriptor open
indefinitely.

If the driver is unbound while the file descriptor is open, iris_remove()
executes and returns, causing devres to free core.

When userspace later closes the file descriptor, the V4L2 core invokes
iris_close(), which attempts to dereference inst->core:

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

Inside iris_remove_session(), this leads to a use-after-free when it locks
inst->core->lock.

Could the allocation of core be managed without devres, or its lifetime tied
to the V4L2 device's release callback?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-glymur-v9-0-8cf2cbe12a07@oss.qualcomm.com?part=2