From: Peilin He<he.peilin@xxxxxxxxxx>
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 <vladimir.oltean@xxxxxxx>
Signed-off-by: Peilin He <he.peilin@xxxxxxxxxx>
Reviewed-by: xu xin <xu.xin16@xxxxxxxxxx>
Signed-off-by: Kun Jiang <jiang.kun2@xxxxxxxxxx>
Cc: Fan Yu <fan.yu9@xxxxxxxxxx>
Cc: Yutan Qiu <qiu.yutan@xxxxxxxxxx>
Cc: Yaxin Wang <wang.yaxin@xxxxxxxxxx>
Cc: tuqiang <tu.qiang35@xxxxxxxxxx>
Cc: Yang Yang <yang.yang29@xxxxxxxxxx>
Cc: ye xingchen <ye.xingchen@xxxxxxxxxx>
Cc: Yunkai Zhang <zhang.yunkai@xxxxxxxxxx>
---
net/dsa/dsa2.c | 7 +++++++
1 file changed, 7 insertions(+)
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);
}
--
2.25.1
