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:   Mon, 2 May 2022 20:34:43 +0200
From:   Francesco Dolcini <francesco.dolcini@...adex.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Francesco Dolcini <francesco.dolcini@...adex.com>,
        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

On Mon, May 02, 2022 at 08:24:53PM +0200, Andrew Lunn wrote:
> > writing to this register could trigger a FEC_ENET_MII interrupt actually
> > creating a race condition with fec_enet_mdio_read() that is called on
> > link change also.
> 
> Another point to consider:
> 
> static int fec_enet_mdio_wait(struct fec_enet_private *fep)
> {
>         uint ievent;
>         int ret;
> 
>         ret = readl_poll_timeout_atomic(fep->hwp + FEC_IEVENT, ievent,
>                                         ievent & FEC_ENET_MII, 2, 30000);
> 
>         if (!ret)
>                 writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
> 
>         return ret;
> }
> 
> An unexpected interrupt will make this exit too early, and the read
> will get invalid data. An unexpected interrupt would not cause a
> timeout here, which is what you are reporting.

yes, I had the exact same thought. Could it be that this creates some
kind of misalignments between the MDIO data and the related FEC
interrupt with the first early exist not triggering any error and the
second one triggering an infinite wait?

I guess I need to sleep on this, in the meantime I have a test running
with the change I described running since a couple of hours.

Francesco