[PATCH 0/2] net: dsa: RCU-protect dsa_ptr in struct net_device

[PATCH 0/2] net: dsa: RCU-protect dsa_ptr in struct net_device

[A. Sverdlin]

From: Alexander Sverdlin <alexander.sverdlin@siemens.com>

There are multiple races of zeroing dsa_ptr in struct net_device (on
shutdown/remove) against asynchronous dereferences all over the net
code. Widespread pattern is as follows:

CPU0					CPU1
if (netdev_uses_dsa())
					dev->dsa_ptr = NULL;
        dev->dsa_ptr->...

RCU-protect the dsa_ptr and ajust the documentation where unsafe patterns
were mentioned.

The first patch has been compiled with ARCH=arm64 and allyesconfig against
v6.11-rc6. Backported to v6.1 and tested on arm64 HW with LAN9303 switch
(where the issue was initially reproduced), including LOCKDEP and
PROVE_RCU. Therefore I will be able to provide v6.1 backport (for -stable
purposes) and later v6.8 backport as well.

Alexander Sverdlin (2):
  net: dsa: RCU-protect dsa_ptr in struct net_device
  docs: net: dsa: RCU protection of dsa_ptr in struct net_device

 Documentation/networking/dsa/dsa.rst        |  18 ++--
 drivers/net/dsa/mt7530.c                    |   3 +-
 drivers/net/dsa/ocelot/felix.c              |   3 +-
 drivers/net/dsa/qca/qca8k-8xxx.c            |   3 +-
 drivers/net/ethernet/broadcom/bcmsysport.c  |   8 +-
 drivers/net/ethernet/mediatek/airoha_eth.c  |   2 +-
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |  22 +++--
 drivers/net/ethernet/mediatek/mtk_ppe.c     |  15 ++-
 include/linux/netdevice.h                   |   2 +-
 include/net/dsa.h                           |  36 +++++--
 include/net/dsa_stubs.h                     |   6 +-
 net/bridge/br_input.c                       |   2 +-
 net/core/dev.c                              |   3 +-
 net/core/flow_dissector.c                   |  19 ++--
 net/dsa/conduit.c                           |  66 ++++++++-----
 net/dsa/dsa.c                               |  19 ++--
 net/dsa/port.c                              |   3 +-
 net/dsa/tag.c                               |   3 +-
 net/dsa/tag.h                               |  19 ++--
 net/dsa/tag_8021q.c                         |  10 +-
 net/dsa/tag_brcm.c                          |   2 +-
 net/dsa/tag_dsa.c                           |   8 +-
 net/dsa/tag_qca.c                           |  10 +-
 net/dsa/tag_sja1105.c                       |  22 +++--
 net/dsa/user.c                              | 104 +++++++++++---------
 net/ethernet/eth.c                          |   2 +-
 26 files changed, 249 insertions(+), 161 deletions(-)

-- 
2.46.0

[PATCH 2/2] docs: net: dsa: RCU protection of dsa_ptr in struct net_device

[A. Sverdlin]

From: Alexander Sverdlin <alexander.sverdlin@siemens.com>

Mention new netdev_uses_dsa_currently() and use rtnl_dereference() for
dsa_ptr access.

Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
---
 Documentation/networking/dsa/dsa.rst | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index 7b2e69cd7ef0..858c6d2842c4 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -255,7 +255,7 @@ Conduit network device (e.g.: e1000e):
 2. net/ethernet/eth.c::
 
           eth_type_trans(skb, dev)
-                  if (dev->dsa_ptr != NULL)
+                  if (netdev_uses_dsa_currently(dev))
                           -> skb->protocol = ETH_P_XDSA
 
 3. drivers/net/ethernet/\*::
@@ -656,14 +656,14 @@ Switch configuration
   represents the index of the user port, and the ``conduit`` argument represents
   the new DSA conduit ``net_device``. The CPU port associated with the new
   conduit can be retrieved by looking at ``struct dsa_port *cpu_dp =
-  conduit->dsa_ptr``. Additionally, the conduit can also be a LAG device where
-  all the slave devices are physical DSA conduits. LAG DSA  also have a
-  valid ``conduit->dsa_ptr`` pointer, however this is not unique, but rather a
-  duplicate of the first physical DSA conduit's (LAG slave) ``dsa_ptr``. In case
-  of a LAG DSA conduit, a further call to ``port_lag_join`` will be emitted
-  separately for the physical CPU ports associated with the physical DSA
-  conduits, requesting them to create a hardware LAG associated with the LAG
-  interface.
+  rtnl_dereference(conduit->dsa_ptr)``. Additionally, the conduit can also be a
+  LAG device where all the slave devices are physical DSA conduits. LAG DSA
+  also have a valid ``conduit->dsa_ptr`` pointer, however this is not unique,
+  but rather a duplicate of the first physical DSA conduit's (LAG slave)
+  ``dsa_ptr``. In case of a LAG DSA conduit, a further call to ``port_lag_join``
+  will be emitted separately for the physical CPU ports associated with the
+  physical DSA conduits, requesting them to create a hardware LAG associated
+  with the LAG interface.
 
 PHY devices and link management
 -------------------------------
-- 
2.46.0

Re: [PATCH 0/2] net: dsa: RCU-protect dsa_ptr in struct net_device

[Jakub Kicinski]

On Tue, 10 Sep 2024 15:03:14 +0200 A. Sverdlin wrote:
> Subject: [PATCH 0/2] net: dsa: RCU-protect dsa_ptr in struct net_device

When you repost please use --subject-prefix="PATCH net v2" 
in git format-patch. The "net" part, specifically, is important,
the patch doesn't apply to net-next so we need to give our bots
a bit of a hint as to which tree this should be (build) tested 
against.
-- 
pw-bot: cr