* [PATCH net] net: dsa: clear devlink port type before unregistering slave netdevs
@ 2021-01-12 0:48 Vladimir Oltean
2021-01-12 18:49 ` Florian Fainelli
0 siblings, 1 reply; 3+ messages in thread
From: Vladimir Oltean @ 2021-01-12 0:48 UTC (permalink / raw
To: David S. Miller, Jakub Kicinski, netdev
Cc: Jiri Pirko, andrew, f.fainelli, vivien.didelot
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>
---
net/dsa/dsa2.c | 4 ++++
1 file changed, 4 insertions(+)
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;
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] net: dsa: clear devlink port type before unregistering slave netdevs
2021-01-12 0:48 [PATCH net] net: dsa: clear devlink port type before unregistering slave netdevs Vladimir Oltean
@ 2021-01-12 18:49 ` Florian Fainelli
2021-01-13 2:54 ` Jakub Kicinski
0 siblings, 1 reply; 3+ messages in thread
From: Florian Fainelli @ 2021-01-12 18:49 UTC (permalink / raw
To: Vladimir Oltean, David S. Miller, Jakub Kicinski, netdev
Cc: Jiri Pirko, andrew, vivien.didelot
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>
Thanks!
--
Florian
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] net: dsa: clear devlink port type before unregistering slave netdevs
2021-01-12 18:49 ` Florian Fainelli
@ 2021-01-13 2:54 ` Jakub Kicinski
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2021-01-13 2:54 UTC (permalink / raw
To: Florian Fainelli, Vladimir Oltean
Cc: David S. Miller, netdev, Jiri Pirko, andrew, vivien.didelot
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!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-01-13 2:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-12 0:48 [PATCH net] net: dsa: clear devlink port type before unregistering slave netdevs Vladimir Oltean
2021-01-12 18:49 ` Florian Fainelli
2021-01-13 2:54 ` Jakub Kicinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.