Re: [PATCH v17 05/11] media: qcom: camss: csiphy: Add support for v2.4.0 two-phase CSIPHY - sashiko-bot

From: sashiko-bot@kernel.org
To: "Hangxiang Ma" <hangxiang.ma@oss.qualcomm.com>
Cc: vkoul@kernel.org, neil.armstrong@linaro.org,
	linux-phy@lists.infradead.org, robh@kernel.org,
	olteanv@gmail.com, media-ci@linuxtv.org,
	devicetree@vger.kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v17 05/11] media: qcom: camss: csiphy: Add support for v2.4.0 two-phase CSIPHY
Date: Tue, 29 Sep 2026 06:17:11 +0000	[thread overview]
Message-ID: <20260929061712.083BD1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260928-kaanapali-camss-v17-5-dcf3fd37f76c@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] media: camss: guaranteed NULL pointer dereference in msm_csiphy_subdev_init()
--

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

media: qcom: camss: csiphy: Add support for v2.4.0 two-phase CSIPHY

Add support for the v2.4.0 two-phase CSIPHY found on Kaanapali, where
the PHY is driven by the standalone phy-qcom-mipi-csi2 DPHY driver
rather than by CAMSS. Add the mipi_csi2_dphy_3nm_kaanapali configuration,
which reuses the x1e80100 lane sequence, clocks and supplies but selects
the v2.4.0 common status offset, adding Kaanapali power domain and
register the "qcom,kaanapali-csi2-phy" compatible.

> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index a528760487bd9..a0cd839c9948a 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -34,6 +34,57 @@
>  
>  static const struct parent_dev_ops vfe_parent_dev_ops;
>  
> +static const struct camss_subdev_resources csiphy_res_kaanapali[] = {
> +	/* CSIPHY0 */
> +	{
> +		.csiphy = {
> +			.id = 0,
> +			.hw_ops = &csiphy_ops_3ph_1_0,
> +			.formats = &csiphy_formats_sdm845
> +		},
> +	},

[Severity: Critical]
Can this result in a NULL pointer dereference during device probe?

Because this Kaanapali resource definition completely omits the .reg array, it
implicitly initializes to NULL. However, it still binds legacy hardware
operations (&csiphy_ops_3ph_1_0) that expect MMIO mappings.

When the CAMSS probe unconditionally iterates over all csiphy_num entries
and invokes msm_csiphy_subdev_init() for each, it attempts to map the legacy
register space:

drivers/media/platform/qcom/camss/camss-csiphy.c:msm_csiphy_subdev_init() {
    ...
    csiphy->base = devm_platform_ioremap_resource_byname(pdev, res->reg[0]);
    ...
}

Because .reg was omitted, res->reg[0] is NULL, which propagates down to
platform_get_resource_byname(). Since Kaanapali defines other named resources,
evaluating strcmp(r->name, NULL) with a NULL name argument can cause a crash.

Does the standalone PHY need to be consumed via the kernel's generic PHY
framework (e.g., devm_phy_get) instead of retaining legacy CAMSS ops that
assume direct MMIO control?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928-kaanapali-camss-v17-0-dcf3fd37f76c@oss.qualcomm.com?part=5

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