Re: [PATCH v2 05/12] media: iris: Route buffers to the matching context bank device

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] In `iris_vdec_inst_init`, the memory allocations for `fmt_src` and `fmt_dst` are not checked for failure, leading to an immediate NULL pointer dereference on allocation failure.
- [High] The `fmt_src` and `fmt_dst` allocations are leaked if initialization fails during `iris_open` or at the end of the inst_init functions.
- [High] Use-After-Free of `struct iris_core` due to `devm_kzalloc` mismatch with V4L2 file descriptor lifetimes.
--

--- Patch [5]: [PATCH v2 05/12] media: iris: Route buffers to the matching context bank device ---
commit 727c95dcb8829de9509a16605cf4666c250086ac
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>

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
is restricted to use 0-600MB of IOVA space, while the pixel stream can
address the full range.

> 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

[ ... ]

While reviewing this driver, I noticed a few pre-existing regressions in
the related code that might need attention:

In drivers/media/platform/qcom/iris/iris_vdec.c:iris_vdec_inst_init():

> 	inst->fmt_src = kzalloc_obj(*inst->fmt_src);
> 	inst->fmt_dst = kzalloc_obj(*inst->fmt_dst);
> 
> 	inst->fw_min_count = 0;
> 
> 	f = inst->fmt_src;
> 	f->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;

[Severity: Critical]
This isn't a bug introduced by this patch, but could kzalloc_obj() return
a null pointer under memory pressure and cause a dereference when setting
f->type?

In drivers/media/platform/qcom/iris/iris_vidc.c:iris_open():

> fail_m2m_ctx_release:
> 	v4l2_m2m_ctx_release(inst->m2m_ctx);
> fail_m2m_release:
> 	v4l2_m2m_release(inst->m2m_dev);
> fail_v4l2_fh_deinit:
> 	iris_v4l2_fh_deinit(inst, filp);
> 	mutex_destroy(&inst->ctx_q_lock);
> 	mutex_destroy(&inst->lock);
> 	kfree(inst);
> 
> 	return ret;

[Severity: High]
This isn't a bug introduced by this patch, but does this error path leave
the memory allocated for inst->fmt_src and inst->fmt_dst leaked?

In drivers/media/platform/qcom/iris/iris_probe.c:iris_probe():

> 	core = devm_kzalloc(&pdev->dev, sizeof(*core), GFP_KERNEL);

[Severity: High]
This isn't a bug introduced by this patch, but since core is allocated with
devm_kzalloc, its lifetime is tied to device binding.

If the driver is unbound but a file descriptor remains open, can a
subsequent close() lead to a use-after-free?

For example, when iris_check_num_queued_internal_buffers() is called
during close:

> 	const struct iris_firmware_data *firmware_data = inst->core->iris_firmware_data;

Could inst->core have already been freed by devres at this point?

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