| Message ID | 20210112004831.3778323-1-olteanv@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [net] net: dsa: clear devlink port type before unregistering slave netdevs | expand |
On Tue, 12 Jan 2021 10:49:21 -0800 Florian Fainelli wrote: > On 1/11/21 4:48 PM, Vladimir Oltean wrote: > > From: Vladimir Oltean <vladimir.oltean@nxp.com> > > > > Florian reported a use-after-free bug in devlink_nl_port_fill found with > > KASAN: > > > > (devlink_nl_port_fill) > > (devlink_port_notify) > > (devlink_port_unregister) > > (dsa_switch_teardown.part.3) > > (dsa_tree_teardown_switches) > > (dsa_unregister_switch) > > (bcm_sf2_sw_remove) > > (platform_remove) > > (device_release_driver_internal) > > (device_links_unbind_consumers) > > (device_release_driver_internal) > > (device_driver_detach) > > (unbind_store) > > > > Allocated by task 31: > > alloc_netdev_mqs+0x5c/0x50c > > dsa_slave_create+0x110/0x9c8 > > dsa_register_switch+0xdb0/0x13a4 > > b53_switch_register+0x47c/0x6dc > > bcm_sf2_sw_probe+0xaa4/0xc98 > > platform_probe+0x90/0xf4 > > really_probe+0x184/0x728 > > driver_probe_device+0xa4/0x278 > > __device_attach_driver+0xe8/0x148 > > bus_for_each_drv+0x108/0x158 > > > > Freed by task 249: > > free_netdev+0x170/0x194 > > dsa_slave_destroy+0xac/0xb0 > > dsa_port_teardown.part.2+0xa0/0xb4 > > dsa_tree_teardown_switches+0x50/0xc4 > > dsa_unregister_switch+0x124/0x250 > > bcm_sf2_sw_remove+0x98/0x13c > > platform_remove+0x44/0x5c > > device_release_driver_internal+0x150/0x254 > > device_links_unbind_consumers+0xf8/0x12c > > device_release_driver_internal+0x84/0x254 > > device_driver_detach+0x30/0x34 > > unbind_store+0x90/0x134 > > > > What happens is that devlink_port_unregister emits a netlink > > DEVLINK_CMD_PORT_DEL message which associates the devlink port that is > > getting unregistered with the ifindex of its corresponding net_device. > > Only trouble is, the net_device has already been unregistered. > > > > It looks like we can stub out the search for a corresponding net_device > > if we clear the devlink_port's type. This looks like a bit of a hack, > > but also seems to be the reason why the devlink_port_type_clear function > > exists in the first place. > > > > Fixes: 3122433eb533 ("net: dsa: Register devlink ports before calling DSA driver setup()") > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> > Tested-by: Florian fainelli <f.fainelli@gmail.com> > Reported-by: Florian Fainelli <f.fainelli@gmail.com> Applied, thanks!
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index c05b1b54c4f7..839c3a770d23 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -353,9 +353,13 @@ static int dsa_port_devlink_setup(struct dsa_port *dp) static void dsa_port_teardown(struct dsa_port *dp) { + struct devlink_port *dlp = &dp->devlink_port; + if (!dp->setup) return; + devlink_port_type_clear(dlp); + switch (dp->type) { case DSA_PORT_TYPE_UNUSED: break;