Re: [PATCH v9 2/6] phy: qcom: qmp-pcie: Add QMP PCIe Multi-PHY driver

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing error check for of_property_count_strings() leads to passing a negative value as array size to devm_kcalloc().
--

--- Patch [2]: [PATCH v9 2/6] phy: qcom: qmp-pcie: Add QMP PCIe Multi-PHY driver ---
commit 5143b0d54dc0134303885f90816364c6021fa993
Author: Qiang Yu <[email protected]>

phy: qcom: qmp-pcie: Add QMP PCIe Multi-PHY driver

This commit adds a dedicated driver for QMP PCIe PHY hardware blocks that
support multiple link topologies (bifurcation). It reads the current link mode
from a TCSR register, and dynamically instantiates the required sub-PHYs,
clock providers, and #phy-cells to expose individual sub-PHYs by index.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c
> new file mode 100644
> index 0000000000000..e93cba4369fb1
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c

[ ... ]

> +static int qmp_pcie_multiphy_register_clocks(struct device *dev,
> +					     struct device_node *np,
> +					     struct qmp_pcie_multiphy *qmp_data)
> +{
> +	int num_pipe_outputs;
> +	int i, ret;
> +
> +	num_pipe_outputs = of_property_count_strings(np, "clock-output-names");
> +
> +	qmp_data->num_pipe_outputs = num_pipe_outputs;
> +	qmp_data->pipe_out_clks = devm_kcalloc(dev, num_pipe_outputs,
> +					       sizeof(*qmp_data->pipe_out_clks),
> +					       GFP_KERNEL);

[Severity: Medium]
If the clock-output-names property is missing from the device tree,
of_property_count_strings() will return a negative error code (such as
-EINVAL). 

Will passing this negative value as the array size to devm_kcalloc() cause it
to implicitly cast to a massive size_t value? While the allocator will likely
fail safely and return NULL, it seems this would cause the driver to return
-ENOMEM instead of propagating the actual error code.

Should we add a check to verify num_pipe_outputs > 0 before attempting the
allocation?

> +	if (!qmp_data->pipe_out_clks)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < num_pipe_outputs; i++) {
> +		ret = __phy_pipe_clk_register(dev, np, i,
> +					      &qmp_data->pipe_out_clks[i]);
> +		if (ret)
> +			return ret;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2