Re: [PATCH] net: mdiobus: Fix memory leak in __mdiobus_register

From: Pavel Skripkin
Date: Tue Sep 28 2021 - 05:45:35 EST

On 9/28/21 12:26, Dan Carpenter wrote:
On Tue, Sep 28, 2021 at 11:55:49AM +0300, Dan Carpenter wrote:
I don't have a solution. I have commented before that I hate kobjects
for this reason because they lead to unfixable memory leaks during
probe. But this leak will only happen with fault injection and so it
doesn't affect real life. And even if it did, a leak it preferable to a

The fix for this should have gone in devm_of_mdiobus_register() but it's
quite tricky.

106 int devm_of_mdiobus_register(struct device *dev, struct mii_bus *mdio,
107 struct device_node *np)
108 {
109 struct mdiobus_devres *dr;
110 int ret;
112 if (WARN_ON(!devres_find(dev, devm_mdiobus_free,
113 mdiobus_devres_match, mdio)))
114 return -EINVAL;

This leaks the bus. Fix this leak by calling mdiobus_release(mdio);

116 dr = devres_alloc(devm_mdiobus_unregister, sizeof(*dr), GFP_KERNEL);
117 if (!dr)
118 return -ENOMEM;

Fix this path by calling mdiobus_release(mdio);

120 ret = of_mdiobus_register(mdio, np);
121 if (ret) {

Ideally here we can could call device_put(mdio), but that won't work for
the one error path that occurs before device_initialize(). /* Do not
continue if the node is disabled */.

Maybe the code could be modified to call device_initialize() on the
error path? Sort of ugly but it would work.

122 devres_free(dr);
123 return ret;
124 }
126 dr->mii = mdio;
127 devres_add(dev, dr);
128 return 0;
129 }

Then audit the callers, and there is only one which references the
mdio_bus after devm_of_mdiobus_register() fails. It's
realtek_smi_setup_mdio(). Modify that debug statement.

Thank you, Dan, for analysis, and it sounds reasonable to me.

Back to bug reported by syzbot: error happened in other place:

int __mdiobus_register(struct mii_bus *bus, struct module *owner)
phydev = mdiobus_scan(bus, i); <-- here
if (IS_ERR(phydev) && (PTR_ERR(phydev) != -ENODEV)) {
err = PTR_ERR(phydev);
goto error;

(You can take a look at the log [1] you won't find error message about mii_bus registration failure. I found this place while debugging locally)

So, Yanfei's patch is completely unrelated to bug reported by syzkaller and Reported-by tag is also wrong.

Can you, please, take a look at [2]. I think, I found the root case of the reported bug. Thank you :)



With regards,
Pavel Skripkin