Re: [PATCH v10 04/14] media: iris: Add helper to select relevant 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:
- [Critical] Severe Use-After-Free in firmware response handler because `v4l2_m2m_ctx_release` frees the context before the firmware session is closed.
- [High] Unprotected list iteration on V4L2 ready queues causes data races and list corruption.
--

--- Patch [4]: [PATCH v10 04/14] media: iris: Add helper to select relevant context bank device ---
commit 694c94b16bf3fad4b4fbf3ea9de787f2ecf1a955
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
Subject: media: iris: Add helper to select relevant context bank device

This commit introduces a helper to associate context bank devices with
respective buffers in the iris driver. It also adds a fallback to the
core device when the platform describes the IOMMU on the parent node.

> diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c
> index 14d63dc76c9ba..26df872fb4fa1 100644
> --- a/drivers/media/platform/qcom/iris/iris_vidc.c
> +++ b/drivers/media/platform/qcom/iris/iris_vidc.c
[ ... ]
> @@ -121,7 +121,7 @@ iris_m2m_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_
>  	dst_vq->drv_priv = inst;
>  	dst_vq->buf_struct_size = sizeof(struct iris_buffer);
>  	dst_vq->min_reqbufs_allocation = MIN_BUFFERS;
> -	dst_vq->dev = inst->core->dev;
> +	dst_vq->dev = iris_get_cb_dev(inst, BUF_OUTPUT);
>  	dst_vq->lock = &inst->ctx_q_lock;
>  
>  	return vb2_queue_init(dst_vq);

[Severity: Critical]
This isn't a bug introduced by this patch, but is there a use-after-free
regression during device closure in iris_close()?

When a userspace application closes the device FD, iris_close() invokes
v4l2_m2m_ctx_release() which immediately frees the memory-to-memory context:

drivers/media/platform/qcom/iris/iris_vidc.c:iris_close() {
    ...
    v4l2_m2m_ctx_release(inst->m2m_ctx);
    ...
}

However, this happens before the firmware session is closed and before the
instance is removed from the global core->instances list. If the firmware
asynchronously sends a response (e.g., HFI_CMD_BUFFER) during this window,
can the IRQ handler in iris_hfi_gen2_handle_output_buffer() look up the
instance and dereference the freed inst->m2m_ctx?

drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c:iris_hfi_gen2_handle_output_buffer() {
    ...
    struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
    ...
    v4l2_m2m_for_each_dst_buf_safe(m2m_ctx, m2m_buffer, n) {
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but could this list iteration cause
a data race resulting in corruption of the V4L2 ready queues?

In driver functions like iris_hfi_gen2_handle_dequeue_buffers(), the ready
queue is iterated without holding the queue-specific rdy_spinlock:

drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c:iris_hfi_gen2_handle_dequeue_buffers() {
    ...
    v4l2_m2m_for_each_src_buf_safe(m2m_ctx, buffer, n) {
    ...
}

Concurrently, userspace operations like VIDIOC_QBUF can call
v4l2_m2m_buf_queue(), which modifies the list under rdy_spinlock:

drivers/media/v4l2-core/v4l2-mem2mem.c:v4l2_m2m_buf_queue() {
    ...
    spin_lock_irqsave(&q_ctx->rdy_spinlock, flags);
    list_add_tail(&b->list, &q_ctx->rdy_queue);
    ...
}

Since the driver's lockless list iteration races directly with list
modifications from the V4L2 core, might this lead to corrupted list pointers
when QBUF or STREAMOFF execute concurrently with firmware responses?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726-glymur-v10-0-de451559e88b@oss.qualcomm.com?part=4