netdev - Re: [PATCH net-next 0/2] net: dsa: realtek: fix PHY register read corruption

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:   Thu, 17 Feb 2022 13:12:19 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Alvin Šipraga <ALSI@...g-olufsen.dk>
Cc:     Luiz Angelo Daros de Luca <luizluca@...il.com>,
        Alvin Šipraga <alvin@...s.dk>,
        Linus Walleij <linus.walleij@...aro.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>,
        Michael Rasmussen <MIR@...g-olufsen.dk>,
        Arınç ÜNAL <arinc.unal@...nc9.com>,
        "open list:NETWORKING DRIVERS" <netdev@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 0/2] net: dsa: realtek: fix PHY register read
 corruption

> Thank you Andrew for the clear explanation.
> 
> Somewhat unrelated to this series, but are you able to explain to me the
> difference between:
> 
> 	mutex_lock(&bus->mdio_lock);
> and
> 	mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
> 
> While looking at other driver examples I noticed the latter form quite a
> few times too.

This is to do with the debug code for checking for deadlocks,
CONFIG_PROVE_LOCKING. When that feature is enables, each lock/unlock
of a mutex is tracked, and a list is made of what other locks are also
taken, and the order. The code can find deadlocks where one thread
takes A then B, while another thread takes B and then A. It can also
detect when a thread takes lock A and then tries to take lock A again.

Rather than track each individual mutex, it uses classes of mutex. So
bus->mdio_lock is a class of mutex. The code simply tracks that a
bus->mdio_lock has been taken, not a specific bus->mdio_lock. That is
generally sufficient, but not always. The mv88e6xxx switch is like
many switches, accessed over MDIO. But the mv88e6xxx switch offers an
MDIO bus, and there is an MDIO bus driver inside the mv88e6xxx
driver. So you have nested MDIO calls. So this debug code seems the
same class of mutex being taken twice, and thinks it is a
deadlock. You can tell it that nested MDIO calls are actually O.K, it
won't deadlock.

      Andrew