linux-kernel - Re: [[RFC PATCH v4 net-next] 0/2] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

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: <20230912142644.u4sdkveei3e5hwaf@skbuf>
Date:   Tue, 12 Sep 2023 17:26:44 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Lukasz Majewski <lukma@...x.de>
Cc:     Tristram.Ha@...rochip.com, Eric Dumazet <edumazet@...gle.com>,
        Andrew Lunn <andrew@...n.ch>, davem@...emloft.net,
        Woojung Huh <woojung.huh@...rochip.com>,
        Oleksij Rempel <o.rempel@...gutronix.de>,
        Florian Fainelli <f.fainelli@...il.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, UNGLinuxDriver@...rochip.com,
        Oleksij Rempel <linux@...pel-privat.de>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [[RFC PATCH v4 net-next] 0/2] net: dsa: hsr: Enable HSR HW
 offloading for KSZ9477

On Tue, Sep 12, 2023 at 04:03:26PM +0200, Lukasz Majewski wrote:
> > In any case, as long as it's the DSA master's address that we program
> > to hardware, then I see it as inevitable to add a new struct
> > dsa_switch_ops :: master_addr_change() function
> 
> Please correct my understanding. The above change would affect the
> whole DSA subsystem. It would require to have the core DSA modified and
> would affect its operation?

Uhm, yes, that would be the idea. If we were going to track changes to
the DSA master's MAC address, we should do it from the DSA framework
which has the existing netdev notifier listener infrastructure in place.

> > Or you can argue that dragging the DSA master into the discussion
> > about how we should program REG_SW_MAC_ADDR_0 is a complication.
> 
> Yes, it is IMHO the complication.

Ok, it's a point of view.

> git grep -n "REG_SW_MAC_ADDR_0"
> drivers/net/dsa/microchip/ksz8795_reg.h:326:#define REG_SW_MAC_ADDR_0
>         0x68 
> drivers/net/dsa/microchip/ksz9477.c:1194:
>      ksz_write8(dev, REG_SW_MAC_ADDR_0 + i,
> 
> drivers/net/dsa/microchip/ksz9477_reg.h:169:#define REG_SW_MAC_ADDR_0
> 0x0302
> 
> In the current net-next the REG_SW_MAC_ADDR_0 is altered used (the only
> usage are now with mine patches on ksz9477).
> 
> To sum up:
> 
> 1. Up till now in (net-next) REG_SW_MAC_ADDR_0 is ONLY declared for
> Microchip switches. No risk for access - other than HSR patches.

I know (except for Oleksij's WoL patches, which will eventually be
resubmitted).

> 2. The MAC address alteration of DSA master and propagation to slaves
> is a generic DSA bug.

Which can be/will be fixed. The diff I've included in the question to
Jakub closes it, in fact.

> Considering the above - the HSR implementation is safe (to the extend
> to the whole DSA subsystem current operation). Am I correct?

If we exclude the aforementioned bug (which won't be a bug forever),
there still exists the case where the MAC address of a DSA user port can
be changed. The HSR driver has a NETDEV_CHANGEADDR handler for this as
well, and updates its own MAC address to follow the port. If that is
allowed to happen after the offload, currently it will break the offload.