Re: [PATCH v21 06/14] media: qcom: camss: Add Kaanapali compatible

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] media: camss: missing DT port bounds check causes OOB array access on csiphy
- [High] media: camss: dev_pm_domain_detach() NULL pointer dereference on cleanup
--

commit 226a568dbea1991f8bda4c0b73de0c31f612545e
Author: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>

media: qcom: camss: Add Kaanapali compatible

Add CAMSS_KAANAPALI enum, Kaanapali compatible and Kaanapali CAMSS driver
private data. The private data currently includes only basic information.
Later changes will add CSIPHY, TPG, CSID and VFE resources.

> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index c54185c93a8a9..fca3de214a922 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c

[ ... ]

> @@ -5576,6 +5590,13 @@ static void camss_remove(struct platform_device *pdev)
>  	camss_genpd_cleanup(camss);
>  }
>  
> +static const struct camss_resources kaanapali_resources = {
> +	.version = CAMSS_KAANAPALI,
> +	.pd_name = "top",
> +	.icc_res = icc_res_kaanapali,
> +	.icc_path_num = ARRAY_SIZE(icc_res_kaanapali),
> +};
> +

[Severity: High]
Can this empty csiphy configuration lead to an out-of-bounds array access on
the csiphy array?

In camss_parse_endpoint_node(), the device tree port number is assigned
directly to the interface ID without bounds checking against
camss->res->csiphy_num:

	csd->interface.csiphy_id = vep.base.port;

Since kaanapali_resources omits csiphy_num, it defaults to 0. This causes
the camss->csiphy array to be dynamically allocated as a ZERO_SIZE_PTR.

If a device tree specifies an endpoint (even port = <0>), it evaluates to an
out-of-bounds access when the async subdevice binds in
camss_subdev_notifier_bound():

	u8 id = csd->interface.csiphy_id;
	struct csiphy_device *csiphy = &camss->csiphy[id];

	csiphy->cfg.csi2 = &csd->interface.csi2;

Could this cause a regression leading to a kernel panic if the driver
processes a device tree containing endpoints for this compatible?

[Severity: High]
Can omitting vfe_num here lead to a NULL pointer dereference during power
domain cleanup?

During probe failure or driver removal, camss_genpd_cleanup() handles power
domain cleanup:

	if (camss->genpd_num == 1)
		return;

	...
	dev_pm_domain_detach(camss->genpd, true);

If the driver is booted with a device tree lacking PM domains, genpd_num
could be 0. This means camss_genpd_cleanup() will not exit early.

If camss->genpd was never successfully attached (for example, due to having
zero VFEs or no PM domains configured), it remains NULL.

Because dev_pm_domain_detach() lacks a check for a NULL dev pointer before
dereferencing dev->pm_domain, could this unconditional call cause a regression
when cleaning up?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011-kaanapali-camss-v21-0-04fd28d815c6@oss.qualcomm.com?part=6