netdev - Re: [PATCHi v2] net: mdiobus: fix device unregistering in mdiobus

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e1a1251a-9660-af82-2bf5-e4c664dde031@gmail.com>
Date:   Fri, 28 Aug 2020 21:27:08 +0200
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Sascha Hauer <s.hauer@...gutronix.de>
Cc:     netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>, kernel@...gutronix.de
Subject: Re: [PATCHi v2] net: mdiobus: fix device unregistering in
 mdiobus_register

On 28.08.2020 16:15, Sascha Hauer wrote:
> On Thu, Aug 27, 2020 at 10:48:48AM +0200, Heiner Kallweit wrote:
>> On 27.08.2020 09:06, Sascha Hauer wrote:
>>> After device_register has been called the device structure may not be
>>> freed anymore, put_device() has to be called instead. This gets violated
>>> when device_register() or any of the following steps before the mdio
>>> bus is fully registered fails. In this case the caller will call
>>> mdiobus_free() which then directly frees the mdio bus structure.
>>>
>>> Set bus->state to MDIOBUS_UNREGISTERED right before calling
>>> device_register(). With this mdiobus_free() calls put_device() instead
>>> as it ought to be.
>>>
>>> Signed-off-by: Sascha Hauer <s.hauer@...gutronix.de>
>>> ---
>>>
>>> Changes since v1:
>>> - set bus->state before calling device_register(), not afterwards
>>>
>>>  drivers/net/phy/mdio_bus.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>>> index 0af20faad69d..9434b04a11c8 100644
>>> --- a/drivers/net/phy/mdio_bus.c
>>> +++ b/drivers/net/phy/mdio_bus.c
>>> @@ -534,6 +534,8 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
>>>  	bus->dev.groups = NULL;
>>>  	dev_set_name(&bus->dev, "%s", bus->id);
>>>  
>>> +	bus->state = MDIOBUS_UNREGISTERED;
>>> +
>>>  	err = device_register(&bus->dev);
>>>  	if (err) {
>>>  		pr_err("mii_bus %s failed to register\n", bus->id);
>>>
>> LGTM. Just two points:
>> 1. Subject has a typo (PATCHi). And it should be [PATCH net v2], because it's
>>    something for the stable branch.
>> 2. A "Fixes" tag is needed.
> 
> Uh, AFAICT this fixes a patch from 2008, this makes for quite some
> stable updates :)
> 
There's just a handful of LTS kernel versions (oldest is 4.4), therefore it
shouldn't be that bad. But right, for things that have always been like they
are now, sometimes it's tricky to find a proper Fixes tag.

> Sascha
> 
> | commit 161c8d2f50109b44b664eaf23831ea1587979a61
> | Author: Krzysztof Halasa <khc@...waw.pl>
> | Date:   Thu Dec 25 16:50:41 2008 -0800
> | 
> |     net: PHYLIB mdio fixes #2
>