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>] [day] [month] [year] [list]
Message-ID: <202412261916435469rfyTVNfO8PtKWbw6X51-@zte.com.cn>
Date: Thu, 26 Dec 2024 19:16:43 +0800 (CST)
From: <jiang.kun2@....com.cn>
To: <andrew@...n.ch>, <vivien.didelot@...il.com>, <f.fainelli@...il.com>,
        <olteanv@...il.com>, <davem@...emloft.net>, <kuba@...nel.org>,
        <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <gregkh@...uxfoundation.org>, <stable@...r.kernel.org>
Cc: <he.peilin@....com.cn>, <xu.xin16@....com.cn>, <fan.yu9@....com.cn>,
        <qiu.yutan@....com.cn>, <wang.yaxin@....com.cn>,
        <tu.qiang35@....com.cn>, <yang.yang29@....com.cn>,
        <ye.xingchen@....com.cn>, <zhang.yunkai@....com.cn>
Subject: [PATCH stable 5.15] net:dsa:fix the dsa_ptr null pointer dereference

From: Peilin He<he.peilin@....com.cn>

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@....com>
Signed-off-by: Peilin He <he.peilin@....com.cn>
Reviewed-by: xu xin <xu.xin16@....com.cn>
Signed-off-by: Kun Jiang <jiang.kun2@....com.cn>
Cc: Fan Yu <fan.yu9@....com.cn>
Cc: Yutan Qiu <qiu.yutan@....com.cn>
Cc: Yaxin Wang <wang.yaxin@....com.cn>
Cc: tuqiang <tu.qiang35@....com.cn>
Cc: Yang Yang <yang.yang29@....com.cn>
Cc: ye xingchen <ye.xingchen@....com.cn>
Cc: Yunkai Zhang <zhang.yunkai@....com.cn>
---
 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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ