netdev - Re: [BUG] rmmod bfin_mac netdev driver module will cause system crash.

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>] [day] [month] [year] [list]

Message-ID: <386072610811070843q25c3988dx3ef7848c89d09cd3@mail.gmail.com>
Date:	Sat, 8 Nov 2008 00:43:29 +0800
From:	"Bryan Wu" <cooloney@...nel.org>
To:	"Lennert Buytenhek" <buytenh@...tstofly.org>,
	netdev@...r.kernel.org
Subject: Re: [BUG] rmmod bfin_mac netdev driver module will cause system crash.

On Sat, Nov 8, 2008 at 12:41 AM, Lennert Buytenhek
<buytenh@...tstofly.org> wrote:
> On Sat, Nov 08, 2008 at 12:36:08AM +0800, Bryan Wu wrote:
>
>> >> I got a regression since I update our blackfin kernel to the lastest
>> >> 2.6.28-rc2 which includes your phylib patches.
>> >> Please find the tracker here:
>> >> https://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_id=141&tracker_item_id=4595
>> >>
>> >> Briefly, the calling chain is:
>> >> in bfin_mac_remove(), mdiobus_unregister() ->
>> >> device_unregister(&bus->dev); -> put_device() -> kobject_put() ->
>> >> kref_put(&kobj->kref, kobject_release); -> kobject_release() ->
>> >> kobject_cleanup() -> mdiobus_release().
>> >> So in mdiobus_unregister() bus->state = MDIOBUS_UNREGISTERED; but in
>> >> mdiobus_release(), this BUG_ON will fire:
>> >> BUG_ON(bus->state != MDIOBUS_RELEASED);
>> >>
>> >> How about this patch?
>> >>
>> >> ---
>> >> Index: drivers/net/phy/mdio_bus.c
>> >> ===================================================================
>> >> --- drivers/net/phy/mdio_bus.c        (revision 5575)
>> >> +++ drivers/net/phy/mdio_bus.c        (working copy)
>> >> @@ -134,13 +134,16 @@
>> >>       int i;
>> >>
>> >>       BUG_ON(bus->state != MDIOBUS_REGISTERED);
>> >> -     bus->state = MDIOBUS_UNREGISTERED;
>> >>
>> >> +     bus->state = MDIOBUS_RELEASED;
>> >> +
>> >>       device_unregister(&bus->dev);
>> >>       for (i = 0; i < PHY_MAX_ADDR; i++) {
>> >>               if (bus->phy_map[i])
>> >>                       device_unregister(&bus->phy_map[i]->dev);
>> >>       }
>> >> +
>> >> +     bus->state = MDIOBUS_UNREGISTERED;
>> >>  }
>> >>  EXPORT_SYMBOL(mdiobus_unregister);
>> >> ---
>> >
>> > Actually, what I think should be done is this.  Can you try that?
>> >
>> >
>> > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>> > index d0ed1ef..536bda1 100644
>> > --- a/drivers/net/phy/mdio_bus.c
>> > +++ b/drivers/net/phy/mdio_bus.c
>> > @@ -136,7 +136,7 @@ void mdiobus_unregister(struct mii_bus *bus)
>> >        BUG_ON(bus->state != MDIOBUS_REGISTERED);
>> >        bus->state = MDIOBUS_UNREGISTERED;
>> >
>> > -       device_unregister(&bus->dev);
>> > +       device_del(&bus->dev);
>> >        for (i = 0; i < PHY_MAX_ADDR; i++) {
>> >                if (bus->phy_map[i])
>> >                        device_unregister(&bus->phy_map[i]->dev);
>> > --
>>
>> Right, that's the right way. Our tester confirmed that this bug was
>> fixed with this patch.
>> Need I send a patch or you patched this youself?
>
> Can you please confirm on netdev@ that that patch fixes it and then
> I'll submit it with your Tested-by, is that okay with you?
>

Sorry for just clicking reply not reply-all. Tested-by is OK for me
definitely, -:))

-Bryan
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html