netdev - [PATCH net 0/7] More DSA fixes for devres + mdiobus

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

Message-Id: <20220207161553.579933-1-vladimir.oltean@nxp.com>
Date:   Mon,  7 Feb 2022 18:15:46 +0200
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     netdev@...r.kernel.org
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Hauke Mehrtens <hauke@...ke-m.de>,
        Sean Wang <sean.wang@...iatek.com>,
        Landen Chao <Landen.Chao@...iatek.com>,
        DENG Qingfang <dqfext@...il.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        UNGLinuxDriver@...rochip.com,
        Oleksij Rempel <linux@...pel-privat.de>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Laurentiu Tudor <laurentiu.tudor@....com>,
        Rafael Richter <rafael.richter@....de>,
        Daniel Klauer <daniel.klauer@....de>
Subject: [PATCH net 0/7] More DSA fixes for devres + mdiobus_{alloc,register}

The initial patch series "[net,0/2] Fix mdiobus users with devres"
https://patchwork.kernel.org/project/netdevbpf/cover/20210920214209.1733768-1-vladimir.oltean@nxp.com/
fixed some instances where DSA drivers on slow buses (SPI, I2C) trigger
a panic (changed since then to a warn) in mdiobus_free. That was due to
devres calling mdiobus_free() with no prior mdiobus_unregister(), which
again was due to commit ac3a68d56651 ("net: phy: don't abuse devres in
devm_mdiobus_register()") by Bartosz Golaszewski.

Rafael Richter and Daniel Klauer report yet another variation on that
theme, but this time it applies to any DSA switch driver, not just those
on buses which have a "->shutdown() calls ->remove() which unregisters
children" sequence.

Their setup is that of an LX2160A DPAA2 SoC driving a Marvell DSA switch
(MDIO). DPAA2 Ethernet drivers probe on the "fsl-mc" bus
(drivers/bus/fsl-mc/fsl-mc-bus.c). This bus is meant to be the
kernel-side representation of the networking objects kept by the
Management Complex (MC) firmware.

The fsl-mc bus driver has this pattern:

static void fsl_mc_bus_shutdown(struct platform_device *pdev)
{
	fsl_mc_bus_remove(pdev);
}

which proceeds to remove the children on the bus. Among those children,
the dpaa2-eth network driver.

When dpaa2-eth is a DSA master, this removal of the master on shutdown
trips up the device link created by dsa_master_setup(), and as such, the
Marvell switch is also removed.

>From this point on, readers can revisit the description of commits
74b6d7d13307 ("net: dsa: realtek: register the MDIO bus under devres")
5135e96a3dd2 ("net: dsa: don't allocate the slave_mii_bus using devres")

since the prerequisites for the BUG_ON in mdiobus_free() have been
accomplished if there is a devres mismatch between mdiobus_alloc() and
mdiobus_register().

Most DSA drivers have this kind of mismatch, and upon my initial
assessment I had not realized the possibility described above, so I
didn't fix it. This patch series walks through all drivers and makes
them use either fully devres, or no devres.

I am aware that there are DSA drivers that are only known to be tested
with a single DSA master, so some patches are probably overkill for
them. But code is copy-pasted from so many sources without fully
understanding the differences, that I think it's better to not leave an
in-tree source of inspiration that may lead to subtle breakage if not
adapted properly.

Vladimir Oltean (7):
  net: dsa: mv88e6xxx: don't use devres for mdiobus
  net: dsa: ar9331: register the mdiobus under devres
  net: dsa: bcm_sf2: don't use devres for mdiobus
  net: dsa: felix: don't use devres for mdiobus
  net: dsa: seville: register the mdiobus under devres
  net: dsa: mt7530: fix kernel bug in mdiobus_free() when unbinding
  net: dsa: lantiq_gswip: don't use devres for mdiobus

 drivers/net/dsa/bcm_sf2.c                |  7 +++++--
 drivers/net/dsa/lantiq_gswip.c           | 14 +++++++++++---
 drivers/net/dsa/mt7530.c                 |  2 +-
 drivers/net/dsa/mv88e6xxx/chip.c         | 11 ++++++++---
 drivers/net/dsa/ocelot/felix_vsc9959.c   |  4 +++-
 drivers/net/dsa/ocelot/seville_vsc9953.c |  5 +++--
 drivers/net/dsa/qca/ar9331.c             |  3 +--
 7 files changed, 32 insertions(+), 14 deletions(-)

-- 
2.25.1