Re: [PATCH v4 3/3] media: qcom: camss: Add support for X1P42100 CAMSS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] media: camss: mismatched CSID to VFE indices break power on
- [High] media: camss: null dereference during probe with malformed DT on X1P42100
- [High] media: camss: missing bounds check on DT port number leads to OOB write
--

commit ab38e0eb97dc4d60c438b6b2e2fd2590708e04a4
Author: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>

media: qcom: camss: Add support for X1P42100 CAMSS

This commit adds CAMSS support for the Qualcomm X1P42100 (Purwa) SoC.
It defines the X1P42100 hardware configuration, reusing existing CSID/VFE ops
while omitting unused components like CSIPHY1-3 and one IFE.

> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index c54185c93a8a9..7bc6b2e61438d 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -4452,6 +4452,205 @@ static const struct resources_wrapper csid_wrapper_res_x1e80100 = {
>  	.reg = "csid_wrapper",
>  };
>  
> +static const struct camss_subdev_resources csiphy_res_x1p42100[] = {
> +	/* CSIPHY0 */
> +	{
> +		.csiphy = {
> +			.id = 0,
> +			.hw_ops = &csiphy_ops_3ph_1_0,
> +			.formats = &csiphy_formats_sdm845
> +		},
> +	},

[Severity: High]
If camss_detect_legacy_phy() falls back to legacy_phy = true due to a
malformed device tree without a phy remote endpoint, camss_init_subdevices()
invokes msm_csiphy_subdev_init_legacy().

That function attempts to map memory using res->reg[0], but since this
structure does not define .reg, it passes a NULL pointer to
platform_get_resource_byname() which then crashes in strcmp().

Additionally, csiphy_init() lacks a case for CAMSS_X1P42100, which leaves
regs->lane_regs uninitialized in this fallback path.

Will this cause a NULL pointer dereference during driver probe?

> +	/* CSIPHY4 */
> +	{
> +		.csiphy = {
> +			.id = 4,
> +			.hw_ops = &csiphy_ops_3ph_1_0,
> +			.formats = &csiphy_formats_sdm845
> +		},
> +	},
> +};

[Severity: High]
When parsing endpoints from the device tree, camss_parse_endpoint_node()
assigns the device tree port number directly to csd->interface.csiphy_id
without bounds checking against csiphy_num.

If port 4 is used in the device tree to match this hardware ID, csiphy_id
becomes 4. Later, camss_subdev_notifier_bound() uses this unvalidated id
to index into the camss->csiphy array.

Since that array only has 2 elements allocated for X1P42100, could defining
this with id 4 lead to an out-of-bounds memory overwrite during subdevice
binding?

[ ... ]

> +static const struct camss_resources x1p42100_resources = {
> +	.version = CAMSS_X1P42100,
> +	.pd_name = "top",
> +	.csiphy_res = csiphy_res_x1p42100,
> +	.tpg_res = tpg_res_x1e80100,
> +	.csid_res = csid_res_x1p42100,
> +	.vfe_res = vfe_res_x1p42100,
> +	.csid_wrapper_res = &csid_wrapper_res_x1e80100,
> +	.icc_res = icc_res_x1p42100,
> +	.icc_path_num = ARRAY_SIZE(icc_res_x1p42100),
> +	.csiphy_num = ARRAY_SIZE(csiphy_res_x1p42100),
> +	.tpg_num = ARRAY_SIZE(tpg_res_x1e80100),
> +	.csid_num = ARRAY_SIZE(csid_res_x1p42100),
> +	.vfe_num = ARRAY_SIZE(vfe_res_x1p42100),
> +};

[Severity: High]
During initialization, csid->id is assigned the index from the resource
array (0 to 4). When csid_set_power() attempts to power on its paired VFE
using its own ID, it calls vfe_parent_dev_ops_get() which checks
if id < camss->res->vfe_num.

For IDs 3 and 4 (CSID_LITE0 and CSID_LITE1), this bounds check fails and
unconditionally returns -EINVAL, aborting the power-up.

Additionally, this 1:1 index pairing appears to cause CSID1 and CSID2 to
incorrectly power up IFE_LITE_0 and IFE_LITE_1 instead of their expected
components.

Does the mismatch between the number of CSIDs (5) and VFEs (3) break the
power-on sequence for the CSID blocks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-purwa_camss-v4-0-bcf11bf2c650@oss.qualcomm.com?part=3