Re: [PATCH net-next v10 05/10] net: mdio: realtek-rtl9300: Configure hardware polling during probing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Module unload race / Use-After-Free due to devres action attached to class device
--

--- Patch [5]: [PATCH net-next v10 05/10] net: mdio: realtek-rtl9300: Configure hardware polling during probing ---
commit 46ab2c1bebbed4638cc44b5a4c270ec37988afc7
Author: Markus Stockhausen <markus.stockhausen@gmx.de>

net: mdio: realtek-rtl9300: Configure hardware polling during probing
    
This patch allows the polling of Realtek Otto switches to be enabled or
disabled during PHY configuration. This prevents hardware polling from
interfering with complex configuration sequences or firmware loading. Polling
is kept in a consistent state by disabling it initially and toggling it
during PHY attach and detach.
    
Link: https://github.com/openwrt/openwrt/blob/main/target/linux/realtek/files-6.18/drivers/net/mdio/mdio-realtek-otto.c#L818
Link: https://lore.kernel.org/netdev/680696024a8648535ce6dee771fe4de67802e0e8.1769053496.git.daniel@makrotopia.org/

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index 7dcfb72ed8188..f1f15ad23dd3d 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c

[ ... ]

> @@ -628,6 +663,9 @@ static int otto_emdio_probe_one(struct device *dev, struct otto_emdio_priv *priv
>  		bus->write = otto_emdio_write_c22;
>  	}
>  	bus->parent = dev;
> +	bus->notify_phy_attach = otto_emdio_notify_phy_attach;
> +	bus->notify_phy_detach = otto_emdio_notify_phy_detach;
> +
>  	chan = bus->priv;
>  	chan->mdio_bus = mdio_bus;
>  	chan->priv = priv;

[Severity: High]
While reviewing these changes to otto_emdio_probe_one(), I noticed a
potential resource management issue further down in the same function:

drivers/net/mdio/mdio-realtek-rtl9300.c:otto_emdio_probe_one() {
    ...
    kref_get(&priv->kref);
    err = devm_add_action_or_reset(&bus->dev, otto_emdio_cleanup_bus, priv);
    ...
}

Could this sequence lead to a module text use-after-free on unload? 

Because bus->dev is a class device, a local user can extend its lifetime 
indefinitely by keeping its sysfs file open. If an administrator unloads 
the module, the module unloads successfully. 

When the user later closes the sysfs file, won't the release of bus->dev 
trigger the devres action and attempt to execute otto_emdio_cleanup_bus() 
after the module code has already been unmapped?

Should this cleanup action be attached to the parent platform dev instead 
of the class device?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731061400.78301-1-markus.stockhausen@gmx.de?part=5