Re: [PATCH net-next v12 04/11] 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:
- [High] This problem wasn't introduced by this patch, but the patch attempts to fix a devm-related use-after-free by suppressing sysfs unbind, while leaving the exact same use-after-free vulnerability open during probe failure and parent device removal. The underlying structural flaw (mismatched lifetimes between devm-managed `priv` and refcounted `mii_bus`) is not addressed.
--

--- Patch [4]: [PATCH net-next v12 04/11] net: mdio: realtek-rtl9300: suppress sysfs bind/unbind attributes ---
commit d996574f6db0effaeff6b6c77116223c1f0e43d9
Author: Markus Stockhausen <[email protected]>

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

This patch attempts to fix a use-after-free issue when the Realtek MDIO
driver is unbound via sysfs. It suppresses sysfs bind/unbind attributes to
prevent manual unbinding, mitigating the lifetime mismatch between
devm-managed resources and reference-counted MDIO buses.

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 73ac5fdcd2674..1873928b362dd 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
> @@ -830,6 +830,7 @@ static struct platform_driver otto_emdio_driver = {
>  	.driver = {
>  		.name = "mdio-rtl9300",
>  		.of_match_table = otto_emdio_ids,
> +		.suppress_bind_attrs = true,

[Severity: High]
This isn't a bug introduced by this patch, but does this fully mitigate the
use-after-free vulnerability?

While this prevents manual unbinding via sysfs, the same mismatched resource
lifetime issue appears to remain open during probe failures and parent device
removal.

If otto_emdio_probe() fails on a later child node iteration, devres cleanup
is triggered. This unregisters the previously registered MDIO bus and frees
the central priv structure:

drivers/net/mdio/mdio-realtek-rtl9300.c:otto_emdio_probe() {
    ...
    for_each_available_child_of_node(np, child) {
        err = otto_emdio_probe_one(dev, priv, child);
        if (err)
            return err;
    ...
}

If a MAC driver already holds a reference to a PHY device on the registered
bus, it keeps the mii_bus structure alive. If the MAC driver subsequently
accesses the PHY during its teardown, the bus read operation will dereference
the freed priv pointer to acquire the lock:

drivers/net/mdio/mdio-realtek-rtl9300.c:otto_emdio_read_c22() {
    ...
    scoped_guard(mutex, &priv->lock)
        ret = priv->info->read_c22(bus, port, regnum, &value);
    ...
}

Since the underlying structural flaw isn't addressed by suppressing sysfs
unbinding, would it be safer to fix the mismatched lifetimes directly?

>  	},
>  };
>

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