Re: [PATCH v2 07/12] media: iris: Create pixel and non-pixel context banks on VPU3x

[sashiko-bot@kernel.org]

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

New issues:
- [High] Unregistered and freed platform devices (p_dev, np_dev) can be used by concurrent ioctls during driver removal, leading to Use-After-Free.
- [High] Context bank platform devices are missing DMA configuration, causing fatal SMMU faults.

Pre-existing issues:
- [High] Missing cancellation of delayed work sys_error_handler causes Use-After-Free on module removal.
--

--- Patch [7]: [PATCH v2 07/12] media: iris: Create pixel and non-pixel context banks on VPU3x ---
commit 6bf8596f13d0c7afe5ee94339e572204b2a9c6df
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>

media: iris: Create pixel and non-pixel context banks on VPU3x

This commit implements context bank hooks for VPU3x to create "non-pixel"
and "pixel" devices from matching device tree subnodes. It wires these hooks
into the ops for relevant platforms and handles device registration and
unregistration.

> diff --git a/drivers/media/platform/qcom/iris/iris_vpu3x.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
> index 3dad47be78b58..623a0f8a7982c 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu3x.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu3x.c

[ ... ]

> @@ -254,6 +254,40 @@ static void iris_vpu35_power_off_hw(struct iris_core *core)

[ ... ]

> +static int iris_vpu3x_init_cb_devs(struct iris_core *core)
> +{
> +	struct device *dev;
> +
> +	dev = iris_create_cb_dev(core, "non-pixel");

[Severity: High]
Does this new device need explicit DMA configuration?

Since iris_create_cb_dev() creates dummy platform devices for the IOMMU
context banks without a driver, the kernel driver core never calls
really_probe() and never invokes platform_dma_configure(). Without an
explicit call to of_dma_configure() in the driver, the device lacks an IOMMU
domain.

When iris_get_cb_dev() returns these devices for buffer allocation, will the
DMA API return raw physical addresses instead of IOVA, triggering an IOMMU
context fault when the VPU's enabled SMMU streams access this memory?

> +	if (IS_ERR(dev))
> +		return PTR_ERR(dev);
> +
> +	core->np_dev = dev;
> +
> +	dev = iris_create_cb_dev(core, "pixel");
> +	if (IS_ERR(dev))
> +		goto unreg_np_dev;
> +
> +	core->p_dev = dev;
> +
> +	return 0;
> +
> +unreg_np_dev:
> +	if (core->np_dev)
> +		platform_device_unregister(to_platform_device(core->np_dev));
> +	core->np_dev = NULL;
> +
> +	return PTR_ERR(dev);
> +}
> +
> +static void iris_vpu3x_deinit_cb_devs(struct iris_core *core)
> +{
> +	if (core->p_dev)
> +		platform_device_unregister(to_platform_device(core->p_dev));
> +	if (core->np_dev)
> +		platform_device_unregister(to_platform_device(core->np_dev));

[Severity: High]
Can concurrent ioctls access these devices after they are unregistered?

If iris_remove() is called while userspace holds a file descriptor open and
executes an ioctl (like VIDIOC_QBUF), this function unregisters and frees
p_dev and np_dev. The pointers in core are not cleared, and there doesn't
appear to be synchronization with active file handles.

Could a concurrent ioctl call iris_get_cb_dev(), retrieve the freed p_dev or
np_dev pointer, and pass it to the DMA API, resulting in a use-after-free of
the device structure?

> +}

[Severity: High]
This isn't a bug introduced by this patch, but does the module removal path
leave delayed work running?

Looking at the removal path, iris_remove() destroys the lock and exits without
cancelling the sys_error_handler delayed work:

drivers/media/platform/qcom/iris/iris_probe.c:iris_remove() {
    ...
    mutex_destroy(&core->lock);
}

If a hardware error schedules core->sys_error_handler with a delay, and
iris_remove() executes immediately after, the devres manager frees the core
structure. When the delayed work executes, it will call
iris_sys_error_handler(), attempting to access the freed core memory.

Should cancel_delayed_work_sync() be called during removal to prevent a
use-after-free of the core structure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-vpu_iommu_iova_handling-v2-0-da52b5228dbd@oss.qualcomm.com?part=7