Re: [PATCH v2 6/8] media: qcom: camss: Add Hawi compatible camss driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] media: camss: NULL deref in legacy init with malformed DT
--

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

media: qcom: camss: Add Hawi compatible camss driver

Add support for Hawi in the camss driver. Hawi reuses the CSIPHY
resource table from Kaanapali and the TPG resource table from
X1E80100.

> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 5db7e12a6f869..664237ad5ce60 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
[ ... ]
> @@ -6086,6 +6099,17 @@ static const struct camss_resources sm8650_resources = {
>  	.vfe_num = ARRAY_SIZE(vfe_res_sm8650),
>  };
>  
> +static const struct camss_resources hawi_resources = {
> +	.version = CAMSS_HAWI,
> +	.pd_name = "top",
> +	.csiphy_res = csiphy_res_kaanapali,

[Severity: High]
Can a malformed device tree without endpoints cause a NULL pointer
dereference when initializing this new compatible?

If a device tree node for Hawi is provided without endpoints,
camss_detect_legacy_phy() in drivers/media/platform/qcom/camss/camss.c
iterates over zero endpoints, leaving camss->legacy_phy = true:

camss_detect_legacy_phy() {
    ...
    camss->legacy_phy = true;

    /* Find first remote-endpoint and determine if its a PHY */
    for_each_endpoint_of_node(camss->dev->of_node, ep) {
    ...
}

camss_init_subdevices() then unconditionally calls
msm_csiphy_subdev_init_legacy(). Since the csiphy_res_kaanapali
structure reused here lacks a .hw_ops definition,
msm_csiphy_subdev_init_legacy() in
drivers/media/platform/qcom/camss/camss-csiphy.c will dereference
a NULL pointer:

msm_csiphy_subdev_init_legacy() {
    ...
    ret = csiphy->res->hw_ops->init(csiphy);
    ...
}

Should there be an explicit check for NULL hw_ops before dereferencing, or
should camss_detect_legacy_phy() handle cases with zero endpoints
differently so it fails gracefully instead of crashing?

> +	.tpg_res = tpg_res_x1e80100,
> +	.icc_res = icc_res_hawi,
> +	.icc_path_num = ARRAY_SIZE(icc_res_hawi),
> +	.csiphy_num = ARRAY_SIZE(csiphy_res_kaanapali),
> +	.tpg_num = ARRAY_SIZE(tpg_res_x1e80100),
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-hawi-maili-camss-v2-0-fc93222da89c@oss.qualcomm.com?part=6

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy