Re: [PATCH v3 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:

New issues:
- [Critical] Context bank devices (np_dev and p_dev) are unregistered and freed during driver removal while V4L2 queues and internal buffers still hold references to them, leading to a Use-After-Free and IOMMU faults in dma_free_attrs().

Pre-existing issues:
- [Critical] V4L2 instances access devres-managed `iris_core` structure after it has been freed during module removal.
--

commit 0b3161aa43cd126ba013789330d51a653b78bf3a
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
Subject: media: iris: Route buffers to the matching context bank device

The VPU issues direct memory accesses through several stream units, requiring
non-pixel streams to use a specific context bank device due to address range
differences. This commit updates the buffer and queue allocation routines
to fetch and use the matching context bank device based on the buffer type.

> 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);

[Severity: Critical]
Does this code introduce a use-after-free regression for the context bank
device?

If userspace keeps the device node open during driver unbind, the context
bank devices might be unregistered and freed. When the file descriptor is
subsequently closed by userspace, tearing down the internal buffers will
invoke dma_free_attrs() with the freed device. 

Since we are using these devices for memory tracking without taking a
reference, can this lead to memory corruption?

> diff --git a/drivers/media/platform/qcom/iris/iris_resources.c b/drivers/media/platform/qcom/iris/iris_resources.c
> index 2c4d34c7bd77d..a6c3df892ca98 100644
> --- a/drivers/media/platform/qcom/iris/iris_resources.c
> +++ b/drivers/media/platform/qcom/iris/iris_resources.c
[ ... ]
> @@ -138,3 +139,43 @@ int iris_disable_unprepare_clock(struct iris_core *core, enum platform_clk_type 
>  
>  	return 0;
>  }
> +
> +struct device *iris_get_cb_dev(struct iris_inst *inst, enum iris_buffer_type buffer_type)
> +{
> +	struct iris_core *core = inst->core;

[Severity: Critical]
This is a pre-existing issue, but can this dereference of inst->core lead
to a use-after-free?

The iris_core structure is devres-managed. If the driver is unbound while
userspace still has the file descriptor open, devres will free the core
structure immediately. 

When the file is closed later, teardown paths that call into functions like
iris_get_cb_dev() will access the freed inst->core structure.

> diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c
> index fcbc60016beec..064cf0a80748d 100644
> --- a/drivers/media/platform/qcom/iris/iris_vidc.c
> +++ b/drivers/media/platform/qcom/iris/iris_vidc.c
> @@ -108,7 +108,7 @@ iris_m2m_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_
>  	src_vq->drv_priv = inst;
>  	src_vq->buf_struct_size = sizeof(struct iris_buffer);
>  	src_vq->min_reqbufs_allocation = MIN_BUFFERS;
> -	src_vq->dev = inst->core->dev;
> +	src_vq->dev = iris_get_cb_dev(inst, BUF_INPUT);
>  	src_vq->lock = &inst->ctx_q_lock;

[Severity: Critical]
Can assigning this dynamically unregistered context bank device to the
queue cause a similar regression?

Like the internal buffers, if the driver unbinds while userspace
holds a file descriptor open, the queue teardown during the deferred
file close might dereference the freed context bank device.

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