Re: [PATCH v5 06/13] media: iris: Route buffers to the matching context bank device
From: sashiko-bot@kernel.org
Date: 2026-09-26 06:49:21
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] media: use-after-free of context bank devices during deferred V4L2 cleanup -- commit 720de65140e46882c5d0651161ef771fcb1189a2 Author: Vikash Garodia [off-list ref] media: iris: Route buffers to the matching context bank device The VPU issues DMA through several SMMU streams, and the hardware does not give every stream the same addressable range. The non-pixel stream cannot address the low 600MB of IOVA space, while the pixel stream can address the full range. Add iris_get_cb_dev(), which maps a buffer type to the owning context bank device. Bitstream and internal buffers (BIN, ARP, COMV, LINE, NON_COMV, PERSIST) belong to the non-pixel device, and uncompressed buffers (DPB, PARTIAL, SCRATCH_1, SCRATCH_2, VPSS) to the pixel device. BUF_INPUT and BUF_OUTPUT depend on direction and are resolved from inst->domain: for a decoder the input is non-pixel and the output pixel, and the other way round for an encoder. Fall back to core->dev whenever the relevant context bank device is absent, so platforms still describing "iommus" on the parent iris node behave exactly as before to maintain backward compatibility.
quoted hunk ↗ jump to hunk
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@@ -531,7 +531,7 @@ static int iris_create_internal_buffer(struct iris_inst *inst, enum iris_buffer_type buffer_type, u32 index) { struct iris_buffers *buffers = &inst->buffers[buffer_type]; - struct iris_core *core = inst->core; + struct device *dev = iris_get_cb_dev(inst, buffer_type);
[Severity: High] Does this acquire the context bank device for DMA mapping without taking a reference to guarantee its lifetime matches the buffer? If iris_remove() explicitly calls platform_device_unregister() on these devices upon sysfs driver unbind, and no standard buffers are active, their refcount can drop to 0 and they will be freed.
struct iris_buffer *buffer; if (!buffers->size)
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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,
[Severity: High] Can this result in a use-after-free of the context bank device during deferred V4L2 cleanup? If standard buffers are active, videobuf2 holds a reference. However, during deferred cleanup in iris_close(), v4l2_m2m_ctx_release() frees standard buffers before iris_destroy_all_internal_buffers() runs. This drops the final refcount, freeing the devices. Will this fetch and dereference the freed device pointer, passing it to dma_free_attrs()? Additionally, does the devm-allocated core also get freed on unbind, making inst->core a dangling pointer here? [ ... ]
quoted hunk ↗ jump to hunk
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);
[Severity: High] Does this assign a manually lifetime-managed, short-lived device to the long-lived vb2_queue context?
quoted hunk ↗ jump to hunk
src_vq->lock = &inst->ctx_q_lock; ret = vb2_queue_init(src_vq); if (ret)@@ -122,7 +122,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);
[Severity: High] Does this also assign a manually lifetime-managed, short-lived device to the long-lived vb2_queue context? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260926-vpu_iommu_iova_handling-v5-0-0322ca5dc10c@oss.qualcomm.com?part=6