Hi Alexander,
On Tue, Sep 10, 2024 at 03:03:15PM +0200, A. Sverdlin wrote:
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->...
One of the possible crashes:
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010 CPU: 0 PID: 12 Comm: ksoftirqd/0 Tainted: G O 6.1.99+ #1 pc : lan9303_rcv lr : lan9303_rcv Call trace: lan9303_rcv dsa_switch_rcv __netif_receive_skb_list_core netif_receive_skb_list_internal napi_gro_receive fec_enet_rx_napi __napi_poll net_rx_action ...
RCU-protect dsa_ptr and use rcu_dereference() or rtnl_dereference() depending on the calling context.
Rename netdev_uses_dsa() into __netdev_uses_dsa_currently() (assumes ether RCU or RTNL lock held) and netdev_uses_dsa_currently() variants which better reflect the uselessness of the function's return value, which becomes outdated right after the call.
Fixes: ee534378f005 ("net: dsa: fix panic when DSA master device unbinds on shutdown") Cc: stable@vger.kernel.org Signed-off-by: Alexander Sverdlin alexander.sverdlin@siemens.com
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 +- 25 files changed, 240 insertions(+), 152 deletions(-)
Thank you for the patch, and I would like you to not give up on it, even if we will go for a different bug fix for 'stable'.
It's just that it makes me a bit uneasy to have this as the bug fix. "Stable" kernels are supposedly named as such because you're not backporting such a major usage pattern change from the core down to all switch drivers and N other Ethernet drivers which you can't even test. The diffstat here says it all, I believe.
This kind of change, in principle, is okay for an early net-next opening. I have it in my tree, have made a few changes to it, will run more tests on the HW I have, and will let you know over the course of the following weeks when it reaches a stage that I'm also comfortable with.