Re: [PATCH net] net: dsa: unbind all switches from tree when DSA master unbinds
From: Vladimir Oltean <olteanv@gmail.com>
Date: 2021-01-12 00:32:31
On Mon, Jan 11, 2021 at 03:40:46PM -0800, Florian Fainelli wrote:
================================================================== [ 61.359856] BUG: KASAN: use-after-free in devlink_nl_port_fill+0x578/0x7a8 [ 61.366847] Read of size 4 at addr c7cf00a8 by task sh/249 [ 61.372413] [ 61.470930] [<c0eb5458>] (devlink_nl_port_fill) from [<c0eb5e20>] (devlink_port_notify+0x80/0x12c) [ 61.490566] [<c0eb5da0>] (devlink_port_notify) from [<c0eb6558>] (devlink_port_unregister+0x5c/0x108) [ 61.505678] [<c0eb64fc>] (devlink_port_unregister) from [<c0feef60>] (dsa_switch_teardown.part.3+0x188/0x18c) [ 61.523621] [<c0feedd8>] (dsa_switch_teardown.part.3) from [<c0fef00c>] (dsa_tree_teardown_switches+0xa8/0xc4) [ 61.541663] [<c0feef64>] (dsa_tree_teardown_switches) from [<c0fef2cc>] (dsa_unregister_switch+0x124/0x250) [ 61.557328] [<c0fef1a8>] (dsa_unregister_switch) from [<c0ba2f8c>] (bcm_sf2_sw_remove+0x98/0x13c) [ 61.576895] [<c0ba2ef4>] (bcm_sf2_sw_remove) from [<c0a57ae8>] (platform_remove+0x44/0x5c) [ 61.591063] [<c0a57aa4>] (platform_remove) from [<c0a54930>] (device_release_driver_internal+0x150/0x254) [ 61.604438] [<c0a547e0>] (device_release_driver_internal) from [<c0a4d040>] (device_links_unbind_consumers+0xf8/0x12c) [ 61.623162] [<c0a4cf48>] (device_links_unbind_consumers) from [<c0a54864>] (device_release_driver_internal+0x84/0x254) [ 61.645614] [<c0a547e0>] (device_release_driver_internal) from [<c0a54a88>] (device_driver_detach+0x30/0x34) [ 61.663466] [<c0a54a58>] (device_driver_detach) from [<c0a514d8>] (unbind_store+0x90/0x134) [ 61.675605] [<c0a51448>] (unbind_store) from [<c0a50108>] (drv_attr_store+0x50/0x5c) [ 61.810185] [ 61.811715] Allocated by task 31: [ 61.835006] alloc_netdev_mqs+0x5c/0x50c [ 61.839012] dsa_slave_create+0x110/0x9c8 [ 61.843102] dsa_register_switch+0xdb0/0x13a4 [ 61.847548] b53_switch_register+0x47c/0x6dc [ 61.851906] bcm_sf2_sw_probe+0xaa4/0xc98 [ 61.855996] platform_probe+0x90/0xf4 [ 61.859741] really_probe+0x184/0x728 [ 61.863481] driver_probe_device+0xa4/0x278 [ 61.867745] __device_attach_driver+0xe8/0x148 [ 61.872274] bus_for_each_drv+0x108/0x158 [ 61.876361] __device_attach+0x190/0x234 [ 61.880362] device_initial_probe+0x1c/0x20 [ 61.884628] bus_probe_device+0xdc/0xec [ 61.888540] deferred_probe_work_func+0xd4/0x11c [ 61.893242] process_one_work+0x420/0x8f8 [ 61.897341] worker_thread+0x4fc/0x91c [ 61.901174] kthread+0x21c/0x22c [ 61.904478] ret_from_fork+0x14/0x20 [ 61.908125] 0x0 [ 61.910014] [ 61.911543] Freed by task 249: [ 61.934842] kfree+0xbc/0x2a8 [ 61.937877] kvfree+0x34/0x38 [ 61.940913] netdev_freemem+0x30/0x34 [ 61.944661] netdev_release+0x48/0x50 [ 61.948400] device_release+0x5c/0x100 [ 61.952234] kobject_put+0x14c/0x2d8 [ 61.955888] put_device+0x20/0x24 [ 61.959284] free_netdev+0x170/0x194 [ 61.962935] dsa_slave_destroy+0xac/0xb0 [ 61.966936] dsa_port_teardown.part.2+0xa0/0xb4 [ 61.971558] dsa_tree_teardown_switches+0x50/0xc4 [ 61.976354] dsa_unregister_switch+0x124/0x250 [ 61.980887] bcm_sf2_sw_remove+0x98/0x13c [ 61.984977] platform_remove+0x44/0x5c [ 61.988810] device_release_driver_internal+0x150/0x254 [ 61.994129] device_links_unbind_consumers+0xf8/0x12c [ 61.999266] device_release_driver_internal+0x84/0x254 [ 62.004497] device_driver_detach+0x30/0x34 [ 62.008762] unbind_store+0x90/0x134 [ 62.012411] drv_attr_store+0x50/0x5c [ 62.016145] sysfs_kf_write+0x90/0x9c [ 62.019885] kernfs_fop_write+0x178/0x2b4 [ 62.023975] vfs_write+0x19c/0x644 [ 62.027452] ksys_write+0xd4/0x170 [ 62.030930] sys_write+0x18/0x1c [ 62.034232] ret_fast_syscall+0x0/0x2c [ 62.038056] 0xb66eda2c
What is allocated in dsa_slave_create, freed in dsa_slave_destroy and used in devlink_nl_port_fill via devlink_port->type_dev? It's the good old net_device! Either this is a devlink design bug or we're all holding it wrong. In mlxsw, mlxsw_core_port_fini (which calls devlink_port_unregister) is called after unregister_netdev && free_netdev, just like in DSA's case. Apparently devlink_port_unregister notifies of port deletion over netlink, and accesses the freshly freed net_device in the meanwhile. Adding Jiri to figure out which one it is.