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

From: Xu, Yanfei
Date: Tue Sep 28 2021 - 22:06:24 EST




On 9/28/21 11:48 PM, Dongliang Mu wrote:
[Please note: This e-mail is from an EXTERNAL e-mail address]

On Tue, Sep 28, 2021 at 11:41 PM Russell King (Oracle)
<linux@xxxxxxxxxxxxxxx> wrote:

On Tue, Sep 28, 2021 at 04:52:07PM +0300, Dan Carpenter wrote:
On Tue, Sep 28, 2021 at 01:58:00PM +0100, Russell King (Oracle) wrote:

This thread seems to be getting out of hand.

The thread was closed. We need to revert Yanfei's patch and apply
Pavel's patch. He's going to resend.


Sorry for my patch, it is my bad. :( And thanks for Pavel's v2 which help to revert mine.

Regards,
Yanfei

So, I would suggest a simple fix is to set bus->state to
MDIOBUS_UNREGISTERED immediately _after_ the successful
device_register().

Not after. It has to be set to MDIOBUS_UNREGISTERED if device_register()
fails, otherwise there will still be a leak.

Ah yes, you are correct - the device name may not be freed. Also...

* NOTE: _Never_ directly free @dev after calling this function, even
* if it returned an error! Always use put_device() to give up your
* reference instead.

So yes, we must set to MDIOBUS_UNREGISTERED even if device_register()
fails.


So we have reached an agreement. Pavel's patch fixes the syzbot link
[1], other than Yanfei's patch. However, Yanfei's patch also fixes
another memory link nearby.

Right?

[1] https://syzkaller.appspot.com/bug?id=fa99459691911a0369622248e0f4e3285fcedd97

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!