Re: [PATCH 2/5] media: verisilicon: Don't hold a VPU behind an IOMMU in reset after remove

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] media: verisilicon: null pointer dereference in hantro_probe()
  [1]
--

commit 4e4d9ad2d3b9dfeb29c03b860ceeca44937863cf
Author: Arun Iyengar <aiyengar1990@gmail.com>

media: verisilicon: Don't hold a VPU behind an IOMMU in reset after remove

This commit leaves the resets of a VPU behind an IOMMU deasserted during
probe error cleanup and driver removal so the IOMMU can access its
registers when it subsequently runtime suspends.

> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> index 26e5d5547466e..702d878c7b509 100644
> --- a/drivers/media/platform/verisilicon/hantro_drv.c
> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
> @@ -1069,6 +1069,22 @@ static struct v4l2_m2m_dev *hantro_get_v4l2_m2m_dev(struct hantro_dev *vpu)
>  	return v4l2_m2m_init(&vpu_m2m_ops);
>  }
>  
> +/*
> + * The resets may also cover the bus interface that the IOMMU of the VPU is
> + * accessed through, e.g. on the RK3588 AV1 decoder. The IOMMU is a separate
> + * device which is runtime resumed with the VPU while it is unbound, and
> + * accesses its registers again when it runtime suspends afterwards. Holding
> + * the VPU in reset at that point causes an asynchronous external abort, so
> + * leave the resets deasserted for a VPU behind an IOMMU.
> + */
> +static void hantro_reset_assert(struct hantro_dev *vpu)
> +{
> +	if (device_iommu_mapped(vpu->dev))
> +		return;
> +
> +	reset_control_assert(vpu->resets);
> +}
> +
>  static int hantro_probe(struct platform_device *pdev)
>  {
>  	const struct of_device_id *match;

[Severity: High]
This problem wasn't introduced by this patch, but can hantro_probe() hit a
NULL pointer dereference [1] when dereferencing match->data?

drivers/media/platform/verisilicon/hantro_drv.c:hantro_probe() {
	...
	match = of_match_node(of_hantro_match, pdev->dev.of_node);
	vpu->variant = match->data;
	...
}

If a probe is forced via sysfs bind on a device without a matching OF node,
of_match_node() returns NULL and match->data is dereferenced
unconditionally.

[1] https://sashiko.dev/#/bug/linux-4e7cd977-2daf-4048-ad53-7801b4bba80e

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-hantro-robust-v1-0-6b6269c05bfa@gmail.com?part=2