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

Message-ID: <20210112185405.1c1f24e2@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 12 Jan 2021 18:54:05 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        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 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@....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>

Applied, thanks!