netdev - Re: Possible bug into DSA2 code.

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 for Android: free password hash cracker in your pocket

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

Message-ID: <084b3973-88b9-8c71-50d5-9d773999ad04@enneenne.com>
Date:   Sun, 10 Feb 2019 12:45:36 +0100
From:   Rodolfo Giometti <giometti@...eenne.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: Possible bug into DSA2 code.

On 09/02/2019 20:34, Andrew Lunn wrote:
>> So we I see two possible solutions:
>>
>> 1) having both ds->slave_mii_bus and ds->ops->phy_read already defined is an
>> error, then it must be signaled to the calling code, or
> 
> I don't think we can do that. mv88e6xxx optionally instantiates the
> MDIO busses, depending on what is in device tree. If there is no mdio
> property, we need the DSA core to create an MDIO bus.

OK, but using the following check to know if DSA did such allocation is not 
correct because DSA drivers can allocate it by their own:

static void dsa_switch_teardown(struct dsa_switch *ds)
{
         if (ds->slave_mii_bus && ds->ops->phy_read)
                 mdiobus_unregister(ds->slave_mii_bus);

Maybe can we add a flag to register ds->slave_mii_bus allocation by DSA?

> Looking at the driver, ds->slave_mii_bus is assigned in
> mv88e6xxx_setup().
> 
> We have talked about adding a teardown() to the ops structure. This
> seems like another argument we should do it. The mv88e6xxx_teardown()
> can set ds->slave_mii_bus back to NULL, undoing what it did in the
> setup code.

This seems reasonable to me, but in this case you have to call teardown() 
operation before calling mdiobus_unregister() into dsa_switch_teardown() or we 
still have the problem...

Ciao,

Rodolfo

-- 
GNU/Linux Solutions                  e-mail: giometti@...eenne.com
Linux Device Driver                          giometti@...ux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti