[stable,5.15] net:dsa:fix the dsa_ptr null pointer dereference

Commit Message

[e-mail] Dec. 26, 2024, 11:16 a.m. UTC

From: Peilin He<[e-mail]>

Upstream commit 6c24a03a61a2 ("net: dsa: improve shutdown sequence")

Issue
=====
Repeatedly accessing the DSA Ethernet controller via the ethtool command,
followed by a system reboot, may trigger a DSA null pointer dereference,
causing a kernel panic and preventing the system from rebooting properly.
This can lead to data loss or denial-of-service, resulting in serious
consequences.

The following is the panic log:
[  172.523467] Unable to handle kernel NULL pointer dereference at virtual
address 0000000000000020
[  172.706923] Call trace:
[  172.709371]  dsa_master_get_sset_count+0x24/0xa4
[  172.714000]  ethtool_get_drvinfo+0x8c/0x210
[  172.718193]  dev_ethtool+0x780/0x2120
[  172.721863]  dev_ioctl+0x1b0/0x580
[  172.725273]  sock_do_ioctl+0xc0/0x100
[  172.728944]  sock_ioctl+0x130/0x3c0
[  172.732440]  __arm64_sys_ioctl+0xb4/0x100
[  172.736460]  invoke_syscall+0x50/0x120
[  172.740219]  el0_svc_common.constprop.0+0x4c/0xf4
[  172.744936]  do_el0_svc+0x2c/0xa0
[  172.748257]  el0_svc+0x20/0x60
[  172.751318]  el0t_64_sync_handler+0xe8/0x114
[  172.755599]  el0t_64_sync+0x180/0x184
[  172.759271] Code: a90153f3 2a0103f4 a9025bf5 f9418015 (f94012b6)
[  172.765383] ---[ end trace 0000000000000002 ]---

Root Cause
==========
Based on analysis of the Linux 5.15 stable version, the function
dsa_master_get_sset_count() accesses members of the structure pointed
to by cpu_dp without checking for a null pointer.  If cpu_dp is a
null pointer, this will cause a kernel panic.

	static int dsa_master_get_sset_count(struct net_device *dev, int sset)
	{
		struct dsa_port *cpu_dp = dev->dsa_ptr;
		const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops;
		struct dsa_switch *ds = cpu_dp->ds;
		...
	}

dev->dsa_ptr is set to NULL in the dsa_switch_shutdown() or
dsa_master_teardown() functions. When the DSA module unloads,
dsa_master_ethtool_teardown(dev) restores the original copy of
the DSA device's ethtool_ops using "dev->ethtool_ops =
cpu_dp->orig_ethtool_ops;" before setting dev->dsa_ptr to NULL.
This ensures that ethtool_ops remains accessible after DSA unloads.
However, dsa_switch_shutdown does not restore the original copy of
the DSA device's ethtool_ops, potentially leading to a null pointer
dereference of dsa_ptr and causing a system panic.  Essentially,
when we set master->dsa_ptr to NULL, we need to ensure that
no user ports are making requests to the DSA driver.

Solution
========
The addition of the netif_device_detach() function is to ensure that
ioctls, rtnetlinks and ethtool requests on the user ports no longer
propagate down to the driver - we're no longer prepared to handle them.

Fixes: ee534378f005 ("net: dsa: fix panic when DSA master device unbinds on shutdown")
Suggested-by: Vladimir Oltean <[e-mail]>
Signed-off-by: Peilin He <[e-mail]>
Reviewed-by: xu xin <[e-mail]>
Signed-off-by: Kun Jiang <[e-mail]>
Cc: Fan Yu <[e-mail]>
Cc: Yutan Qiu <[e-mail]>
Cc: Yaxin Wang <[e-mail]>
Cc: tuqiang <[e-mail]>
Cc: Yang Yang <[e-mail]>
Cc: ye xingchen <[e-mail]>
Cc: Yunkai Zhang <[e-mail]>
---
 net/dsa/dsa2.c | 7 +++++++
 1 file changed, 7 insertions(+)

Patch

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 543834e31298..bf384b30ec0a 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -1656,6 +1656,7 @@  EXPORT_SYMBOL_GPL(dsa_unregister_switch);
 void dsa_switch_shutdown(struct dsa_switch *ds)
 {
 	struct net_device *master, *slave_dev;
+	LIST_HEAD(close_list);
 	struct dsa_port *dp;

 	mutex_lock(&dsa2_mutex);
@@ -1665,6 +1666,11 @@  void dsa_switch_shutdown(struct dsa_switch *ds)

 	rtnl_lock();

+	dsa_switch_for_each_cpu_port(dp, ds)
+		list_add(&dp->master->close_list, &close_list);
+
+	dev_close_many(&close_list, true);
+
 	list_for_each_entry(dp, &ds->dst->ports, list) {
 		if (dp->ds != ds)
 			continue;
@@ -1675,6 +1681,7 @@  void dsa_switch_shutdown(struct dsa_switch *ds)
 		master = dp->cpu_dp->master;
 		slave_dev = dp->slave;

+		netif_device_detach(slave_dev);
 		netdev_upper_dev_unlink(master, slave_dev);
 	}