linux-kernel - Re: [PATCH] net: dsa: mv88e6xxx: Fix port_hidden_wait to account for port_base

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

Message-ID: <YmWifeWNSlpNrJkl@lunn.ch>
Date:   Sun, 24 Apr 2022 21:18:21 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Marek Behún <kabel@...nel.org>
Cc:     Nathan Rossi <nathan@...hanrossi.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH] net: dsa: mv88e6xxx: Fix port_hidden_wait to account for
 port_base_addr

On Sun, Apr 24, 2022 at 04:52:28PM +0200, Marek Behún wrote:
> On Sun, 24 Apr 2022 14:17:59 +0000
> Nathan Rossi <nathan@...hanrossi.com> wrote:
> 
> > The other port_hidden functions rely on the port_read/port_write
> > functions to access the hidden control port. These functions apply the
> > offset for port_base_addr where applicable. Update port_hidden_wait to
> > use the port_wait_bit so that port_base_addr offsets are accounted for
> > when waiting for the busy bit to change.
> > 
> > Without the offset the port_hidden_wait function would timeout on
> > devices that have a non-zero port_base_addr (e.g. MV88E6141), however
> > devices that have a zero port_base_addr would operate correctly (e.g.
> > MV88E6390).
> 
> So basically the code is accessing the wrong register for devices with
> non-zero port_base_addr. This means that the patch should have a Fixes
> tag with the commit that introduced this bug, so that it gets
> backported to relevant stable versions.
> 
> Could you resend with Fixes tag?

I think that would be:

Fixes: 609070133aff ("net: dsa: mv88e6xxx: update code operating on hidden registers")

That patch moved the code out of chip.c into port_hidden. At the same
time, the functions got renamed from mv88e6390_hidden_read() to
mv886xxx_hidden_read(). 6390 does have zero-based ports, so was
correct before the rename. But the more generic name suggest it should
take into account devices which do not have zero based ports.

With the fixes tag added and based on met:

Reviewed-by: Andrew Lunn <andrew@...n.ch>

    Andrew