Re: [PATCH net-next v8 07/11] net: pcs: xpcs: add Rockchip RK3568 platform glue driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: pcs: xpcs-rk: UAF in xpcs_rk_create due to TOCTOU and unprotected devres access
--

commit 2e7f490907e83788481369bf43ad35abe0d56504
Author: Coia Prant <coiaprant@gmail.com>

net: pcs: xpcs: add Rockchip RK3568 platform glue driver

This commit introduces a generic platform glue driver for the Synopsys
DesignWare XPCS on Rockchip RK3568 SoCs. It translates standard MDIO
operations into APB3 memory-mapped register accesses by creating a
virtual MDIO bus.

Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf
Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf

> diff --git a/drivers/net/pcs/pcs-xpcs-rk.c b/drivers/net/pcs/pcs-xpcs-rk.c
> new file mode 100644
> index 0000000000000..84f8ea7fa57e8
> --- /dev/null
> +++ b/drivers/net/pcs/pcs-xpcs-rk.c

[ ... ]

> +struct dw_xpcs *xpcs_rk_create(struct device *dev, struct device_node *np)
> +{
> +	struct platform_device *pdev;

[ ... ]

> +	if (!device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_CONSUMER)) {
> +		put_device(&pdev->dev);
> +		return ERR_PTR(-EPROBE_DEFER);
> +	}
> +
> +	device_lock(&pdev->dev);
> +	if (!device_is_bound(&pdev->dev)) {
> +		device_unlock(&pdev->dev);
> +		put_device(&pdev->dev);
> +		return ERR_PTR(-EPROBE_DEFER);
> +	}
> +	device_unlock(&pdev->dev);
> +
> +	pxpcs = platform_get_drvdata(pdev);
> +	if (!pxpcs || !pxpcs->bus) {

[Severity: High]
Can this result in a use-after-free of the supplier devres data?

If the XPCS driver unbinds while a second MAC consumer probes, the newly
created device link will be in the DL_STATE_SUPPLIER_UNBIND state. Because
of this state, device_links_unbind_consumers() ignores it and does not wait.

Since the device_lock() is dropped before calling platform_get_drvdata(),
could the unbind finish and free the pxpcs devres allocation before we
dereference pxpcs->bus below?

CPU1 (Supplier Unbinding)             CPU2 (Consumer Probing)
                                      device_link_add()
                                      device_lock()
                                      device_is_bound() returns true
                                      device_unlock()
unbind completes
devres frees pxpcs
                                      pxpcs = platform_get_drvdata(pdev);
                                      xpcs_create_mdiodev(pxpcs->bus, ...)

> +		put_device(&pdev->dev);
> +		return ERR_PTR(-EPROBE_DEFER);
> +	}
> +
> +	xpcs = xpcs_create_mdiodev(pxpcs->bus, (int)port);
> +	if (IS_ERR(xpcs)) {
> +		put_device(&pdev->dev);
> +		return xpcs;
> +	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260919125119.2107089-1-coiaprant@gmail.com?part=7