Re: [PATCH v4 15/16] media: venus: skip QCM2290 if Iris driver is enabled

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-after-free of `hdev` in the IRQ handler due to incorrect teardown sequence.
--

commit a490235af4b563cc6644730f4c262f25f52e330a
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

media: venus: skip QCM2290 if Iris driver is enabled

As the Iris driver now supports the QCM2290 hardware too, there is a
race between Venus and Iris drivers on binding to the corresponding
device. Follow the approach used by other platforms and skip QCM2290 in
the Venus driver if Iris is enabled.

> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index 243e342b0ae75..3c88594eb1d00 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c

[ ... ]

> @@ -1169,13 +1168,16 @@ static const struct venus_resources qcm2290_res = {
>  	.enc_nodename = "video-encoder",
>  	.min_fw = &min_fw,
>  };
> +#endif
>  
>  static const struct of_device_id venus_dt_match[] = {
>  	{ .compatible = "qcom,msm8916-venus", .data = &msm8916_res, },
>  	{ .compatible = "qcom,msm8939-venus", .data = &msm8939_res, },
>  	{ .compatible = "qcom,msm8996-venus", .data = &msm8996_res, },
>  	{ .compatible = "qcom,msm8998-venus", .data = &msm8998_res, },
> +#if (!IS_ENABLED(CONFIG_VIDEO_QCOM_IRIS))
>  	{ .compatible = "qcom,qcm2290-venus", .data = &qcm2290_res, },
> +#endif
>  	{ .compatible = "qcom,sc7180-venus", .data = &sc7180_res, },
>  	{ .compatible = "qcom,sdm660-venus", .data = &sdm660_res, },
>  	{ .compatible = "qcom,sdm845-venus", .data = &sdm845_res, },

[Severity: High]
This is a pre-existing issue, but could there be a use-after-free risk during
teardown in venus_hfi_destroy()?

In drivers/media/platform/qcom/venus/hfi_venus.c:venus_hfi_destroy(), hdev is
freed and its lock is destroyed before the IRQ is disabled:

void venus_hfi_destroy(struct venus_core *core)
{
	struct venus_hfi_device *hdev = to_hfi_priv(core);

	core->priv = NULL;
	venus_interface_queues_release(hdev);
	mutex_destroy(&hdev->lock);
	kfree(hdev);
	disable_irq(core->irq);
	core->ops = NULL;
}

If a threaded interrupt (venus_isr_thread) is already running or scheduled
concurrently, could it pass the initial if (!hdev) check before core->priv
is set to NULL, and then access the freed hdev structure?

drivers/media/platform/qcom/venus/hfi_venus.c:venus_isr_thread()
{
	struct venus_hfi_device *hdev = to_hfi_priv(core);
	...
	if (!hdev)
		return IRQ_NONE;
	...
	pkt = hdev->pkt_buf;

	while (!venus_iface_msgq_read(hdev, pkt)) {
	...
}

Because disable_irq() waits for currently executing handlers to complete,
should it be called before freeing the structures the handler depends on?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-iris-ar50lt-v4-0-0abfb74d5b3c@oss.qualcomm.com?part=15