Re: [PATCH v3 05/13] media: iris: Add non-pixel and pixel context bank devices

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free of devres-allocated `core` memory on driver unbind via lingering file descriptors.
- [High] Missing `cancel_delayed_work_sync` in driver teardown allows `sys_error_handler` to execute asynchronously on freed memory.
--

commit 5f2788e633116dd856570ea29baabf6ce4157fbe
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>

media: iris: Add non-pixel and pixel context bank devices

The VPU issues DMA through several SMMU streams, and the hardware does
not give every stream the same addressable range. This patch describes
"non-pixel" and "pixel" as separate context bank subnodes. Helper
functions are added to create and clean up these devices during probe
and remove.

> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> index e4acf4a74f944..debd1f0e57038 100644
> --- a/drivers/media/platform/qcom/iris/iris_probe.c
> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
[ ... ]
> @@ -207,6 +268,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 isn't a bug introduced by this patch, but doesn't this leave the
driver vulnerable to a use-after-free if a userspace process has the
video device open during driver unbind?

The core context is allocated with devm_kzalloc() in iris_probe(),
so it will be freed automatically upon return from iris_remove().

However, this function does not wait for active V4L2 file descriptors
to close. If a lingering file descriptor is closed later, iris_close()
will invoke iris_remove_session(), which accesses the freed core
memory and attempts to lock the destroyed mutex:

iris_vidc.c:iris_remove_session() {
    ...
    mutex_lock(&core->lock);
    ...
}

[Severity: High]
This is a pre-existing issue, but is there a missing
cancel_delayed_work_sync() for core->sys_error_handler during teardown?

In iris_probe(), the delayed work is initialized:

    INIT_DELAYED_WORK(&core->sys_error_handler, iris_sys_error_handler);

If a hardware SYS_ERROR interrupt fires near the time of unbind, the
delayed work could be scheduled. Since iris_remove() completes without
cancelling it, the core memory is freed by devres.

The work queue could subsequently execute iris_sys_error_handler()
asynchronously on the freed pointer, attempting to access core->dev
and call iris_core_deinit(core).

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-vpu_iommu_iova_handling-v3-0-2fe43ae45495@oss.qualcomm.com?part=5