Re: [PATCH v4 06/13] media: iris: Route buffers to the matching context bank device

[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 `core` structure and context bank devices during deferred V4L2 file descriptor closure.
- [High] Use-After-Free of `core` structure due to uncanceled `sys_error_handler` delayed work.
--

commit 6eb05ecb82941180acaafce80a0667d695f203de
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>

media: iris: Route buffers to the matching context bank device
    
This commit introduces iris_get_cb_dev() to dynamically route buffers to the
correct context bank device based on their type and domain. It handles the
different addressable IOVA ranges between pixel and non-pixel SMMU streams.
If a relevant context bank device is missing, it falls back to the core
device for backward compatibility.

> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
> index eb8de60c1177f..6e03d13ac1a79 100644
> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
[ ... ]
> @@ -650,10 +650,10 @@ int iris_queue_internal_buffers(struct iris_inst *inst, u32 plane)
>  
>  void iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buffer)
>  {
> -	struct iris_core *core = inst->core;
> +	struct device *dev = iris_get_cb_dev(inst, buffer->type);
>  
>  	list_del(&buffer->list);
> -	dma_free_attrs(core->dev, buffer->buffer_size, buffer->kvaddr,
> +	dma_free_attrs(dev, buffer->buffer_size, buffer->kvaddr,
>  		       buffer->device_addr, buffer->dma_attrs);
>  	kfree(buffer);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but this patch introduces
additional use-after-free vectors by dynamically managing unrefcounted
context bank devices.

If userspace holds a file descriptor open and the driver is unbound,
iris_remove() unregisters the context bank devices (p_dev, np_dev). After
iris_remove() returns, the devres subsystem frees the core structure. Later,
when the userspace application closes the file descriptor, iris_close() is
invoked, which calls into iris_destroy_internal_buffer().

Could this lead to a use-after-free when iris_get_cb_dev() dereferences
inst->core, and subsequently when dma_free_attrs() uses the already freed
dev?

While reviewing this code, I also noticed a pre-existing lifetime issue in
iris_remove():

drivers/media/platform/qcom/iris/iris_probe.c:iris_remove() {
...
	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 if a hardware error occurs,
the driver schedules the sys_error_handler delayed work. If the driver is
unbound before this work executes, iris_remove() completes without calling
cancel_delayed_work_sync(&core->sys_error_handler).

Since the devres subsystem then automatically frees the core structure, can
this cause a use-after-free when the delayed work timer expires and
iris_sys_error_handler() dereferences the freed core pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-vpu_iommu_iova_handling-v4-0-9b9074a73c41@oss.qualcomm.com?part=6