Re: [PATCH net-next v14 06/13] net: mdio: realtek-rtl9300: suppress sysfs bind/unbind attributes

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The proposed mitigation (`suppress_bind_attrs = true`) only prevents manual sysfs unbinding but leaves the identical Use-After-Free vulnerability unhandled during the probe error path.
--

--- Patch [6]: [PATCH net-next v14 06/13] net: mdio: realtek-rtl9300: suppress sysfs bind/unbind attributes ---
commit e83244316d6929d12214fe1f515d997dd87e2a27
Author: Markus Stockhausen <[email protected]>

net: mdio: realtek-rtl9300: suppress sysfs bind/unbind attributes

This commit attempts to mitigate a use-after-free issue by setting
suppress_bind_attrs to true. The driver relies on devm for the central
controller structure, which can be freed early while MAC drivers still
hold references to PHY devices on the bus when unbound via sysfs.

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 87b91aef9b56e..3360cadfb7f32 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
> @@ -839,6 +839,7 @@ static struct platform_driver otto_emdio_driver = {
>  	.driver = {
>  		.name = "mdio-rtl9300",
>  		.of_match_table = otto_emdio_ids,
> +		.suppress_bind_attrs = true,

[Severity: Critical]
This isn't a bug introduced by this patch, but does this mitigation fully
address the load failure scenario mentioned in the commit message?

Looking at otto_emdio_probe(), the driver allocates priv via devm_kzalloc()
and then iterates over child nodes to register an MDIO bus for each:

device_for_each_child_node_scoped(dev, child) {
        err = otto_emdio_probe_one(dev, priv, child);
        if (err)
                return err;
}

If a MAC driver concurrently attaches to a PHY on the first successfully
registered bus, it will hold a reference to the PHY device, keeping the
mii_bus structure alive.

If a subsequent child node fails to probe, otto_emdio_probe() returns an
error and triggers devres cleanup. Devres unregisters the buses but leaves
the mii_bus structure allocated due to the elevated reference count. However,
devres unconditionally frees the priv block.

If the MAC driver later attempts an MDIO operation, such as calling
otto_emdio_read_c22(), won't it dereference the freed priv pointer?

static int otto_emdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
{
        struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
        ...
        scoped_guard(mutex, &priv->lock)
                ret = priv->info->read_c22(bus, port, regnum, &value);
        ...
}

Should this probe error path also be addressed?

>  	},
>  };

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