On Thu, Dec 26, 2024 at 07:16:43PM +0800, jiang.kun2@xxxxxxxxxx wrote:
> 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>
You dropped all the original signed-off-by lines :(
