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] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 11 Feb 2019 09:28:37 -0800
From:   Florian Fainelli <>
To:     Rodolfo Giometti <>,
        Andrew Lunn <>
Cc:     Vivien Didelot <>,
        "David S. Miller" <>,
Subject: Re: Possible bug into DSA2 code.

On 2/10/19 3:45 AM, Rodolfo Giometti wrote:
> 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?

If drivers allocate the slave_mii_bus, or use it as a pointer to their
bus, then they should not be providing a ds->ops->phy_read() callback
since we assume they would have mii_bus::read and mii_bus::write set to
their driver internal version.

>> 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


Powered by blists - more mailing lists