netdev - Re: FEC MDIO read timeout on linkup

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]

Date:   Wed, 4 May 2022 00:17:59 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Francesco Dolcini <francesco.dolcini@...adex.com>
Cc:     netdev@...r.kernel.org, Andy Duan <fugang.duan@....com>,
        Joakim Zhang <qiangqing.zhang@....com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        Fabio Estevam <festevam@...il.com>,
        Tim Harvey <tharvey@...eworks.com>,
        Chris Healy <cphealy@...il.com>
Subject: Re: FEC MDIO read timeout on linkup

> I'm wondering could this be related to
> fec_enet_adjust_link()->fec_restart() during a fec_enet_mdio_read()
> and one of the many register write in fec_restart() just creates the
> issue, maybe while resetting the FEC? Does this makes any sense?

phylib is 'single threaded', in that only one thing will be active at
once for a PHY. While fec_enet_adjust_link() is being called, there
will not be any read/writes occurring for that PHY.

However, each PHY in the system runs on its own. If you have multiple
PHYs sharing one MDIO bus, or an Ethernet switch on the bus, they can
be doing read/writes at the same time.

The mdio bus has a lock which prevents actual transactions on the bus
at the same time. Nothing in phylib means that lock is held when
fec_enet_adjust_link() is called, so another PHY could be making MDIO
transfers. If fec_enet_adjust_link() is going to do bad things with
the MDIO bus master, it needs to hold the MDIO lock while doing it.

    Andrew