Re: [PATCH RFC net-next 4/9] net: pcs: xpcs: add Qualcomm Nord platform support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: pcs: missing bounds check enables out-of-bounds MMIO read/write
--

commit 2b4e5bfd24ebda95c719fb78f892dda332feef24
Author: Mohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>

net: pcs: xpcs: add Qualcomm Nord platform support

Adds Qualcomm Nord platform support by exposing DW XPCS register blocks via
separate direct-MMIO windows. It introduces accessors to translate MMD
accesses to these Nord windows and selects them using the Nord synthetic
XPCS ID.

> diff --git a/drivers/net/pcs/pcs-xpcs-plat.c b/drivers/net/pcs/pcs-xpcs-plat.c
> index e78a233fb198d..e013a5253a226 100644
> --- a/drivers/net/pcs/pcs-xpcs-plat.c
> +++ b/drivers/net/pcs/pcs-xpcs-plat.c
[ ... ]
> @@ -285,7 +298,7 @@ static int xpcs_plat_init_res(struct dw_xpcs_plat *pxpcs)
>  	else
>  		spc_size = pxpcs->reg_width * SZ_2M;
>  
> -	if (resource_size(res) < spc_size) {
> +	if (!pxpcs->ops && resource_size(res) < spc_size) {
>  		dev_err(dev, "Invalid reg-space size\n");
>  		return -EINVAL;
>  	}

[Severity: High]
Does bypassing the resource_size() check for platforms with custom ops in
xpcs_plat_init_res() leave the hardware mapping size unvalidated?

Since xpcs_qcom_ops is set for the Qualcomm Nord platform, this skips
ensuring the physical mapped region is large enough to encompass the largest
possible offset.

> diff --git a/drivers/net/pcs/pcs-xpcs-qcom.c b/drivers/net/pcs/pcs-xpcs-qcom.c
> new file mode 100644
> index 0000000000000..63c95c9514548
> --- /dev/null
> +++ b/drivers/net/pcs/pcs-xpcs-qcom.c
[ ... ]
> +static int xpcs_qcom_reg_offset(int dev, int reg)
> +{
> +	switch (dev) {
[ ... ]
> +	case MDIO_MMD_VEND2:
> +		if (reg >= DW_VENDOR)
> +			return QCOM_XPCS_VR_MII_BASE +
> +			       ((reg - DW_VENDOR) << 2);

[Severity: High]
Is there a bounds check missing for the reg index in xpcs_qcom_reg_offset()?

MDIO registers can be read from userspace up to 0xFFFF via ioctl or ethtool.
With MDIO_MMD_VEND2, a large index like 0xFFFF translates into an offset of
0x24fffc (approx 2.4MB), which may exceed standard hardware mapping slices.

> +
> +		return QCOM_XPCS_SR_MII_BASE + (reg << 2);
> +
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +int xpcs_qcom_reg_read(struct platform_device *pdev, void __iomem *reg_base,
> +		       int dev, int reg)
> +{
> +	int offset, ret;
> +
> +	offset = xpcs_qcom_reg_offset(dev, reg);
> +	if (offset < 0)
> +		return offset;
> +
> +	ret = pm_runtime_resume_and_get(&pdev->dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = readl(reg_base + offset) & 0xffff;

[Severity: High]
Could this readl() in xpcs_qcom_reg_read() trigger an out-of-bounds MMIO
access and a Data Abort?

Because xpcs_plat_init_res() bypassed the mapping size check, and
xpcs_qcom_reg_offset() translates unvalidated register indices into large
offsets, this reads from an unmapped physical address if a large reg value
is provided. (The same question applies to writel() in xpcs_qcom_reg_write()
below this block).

> +
> +	pm_runtime_put(&pdev->dev);
> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-qcom_xpcs_nord_emac-v1-0-4b1c682af70f@oss.qualcomm.com?part=4