[PATCHi v2] net: mdiobus: fix device unregistering in mdiobus_register

[PATCHi v2] net: mdiobus: fix device unregistering in mdiobus_register

[Sascha Hauer]

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@pengutronix.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);
-- 
2.28.0

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

[Heiner Kallweit]

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@pengutronix.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.

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

[Sascha Hauer]

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@pengutronix.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 :)

Sascha

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

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

[Heiner Kallweit]

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@pengutronix.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@pm.waw.pl>
> | Date:   Thu Dec 25 16:50:41 2008 -0800
> | 
> |     net: PHYLIB mdio fixes #2
>