netdev - Re: [PATCH net] net: dsa: clear devlink port type before unregistering slave netdevs

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f2974f4e-41d0-379a-3014-39926cf011a0@gmail.com>
Date:   Tue, 12 Jan 2021 10:49:21 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org
Cc:     Jiri Pirko <jiri@...dia.com>, andrew@...n.ch,
        vivien.didelot@...il.com
Subject: Re: [PATCH net] net: dsa: clear devlink port type before
 unregistering slave netdevs

On 1/11/21 4:48 PM, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@....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@....com>

Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
Tested-by: Florian fainelli <f.fainelli@...il.com>
Reported-by: Florian Fainelli <f.fainelli@...il.com>

Thanks!
-- 
Florian