netdev - Re: [PATCH 06/11] net: dsa: microchip: ksz8795: use phy_port

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

Message-ID: <20201119135909.GU1804098@lunn.ch>
Date:   Thu, 19 Nov 2020 14:59:09 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Michael Grzeschik <mgr@...gutronix.de>
Cc:     netdev@...r.kernel.org, f.fainelli@...il.com, davem@...emloft.net,
        kernel@...gutronix.de, matthias.schiffer@...tq-group.com,
        woojung.huh@...rochip.com, UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH 06/11] net: dsa: microchip: ksz8795: use phy_port_cnt
 where possible

On Thu, Nov 19, 2020 at 09:40:05AM +0100, Michael Grzeschik wrote:
> On Thu, Nov 19, 2020 at 01:29:15AM +0100, Andrew Lunn wrote:
> > >  	case BR_STATE_DISABLED:
> > >  		data |= PORT_LEARN_DISABLE;
> > > -		if (port < SWITCH_PORT_NUM)
> > > +		if (port < dev->phy_port_cnt)
> > >  			member = 0;
> > >  		break;
> > 
> > So this, unlike all the other patches so far, is not obviously
> > correct. What exactly does phy_port_cnt mean? Can there be ports
> > without PHYs? What if the PHYs are external? You still need to be able
> > to change the STP state of such ports.
> 
> The variable phy_port_cnt is referring to all external physical
> available ports, that are not connected internally to the cpu.
> 
> That means that the previous code path was already broken, when stp
> handling should include the cpu_port.

So using DSA names, it is the number of user ports. And the assumption
is, the last port is the CPU port.

Please add to the commit message that this patch fixes the code as
well. That sort of comment helps the reviewer understand why it is not
just an obvious mechanical change.

	Andrew