[Bridge] [PATCH net v2] net: bridge: fix early call to br_stp_change_bridge_id and plug newlink leaks

[David Miller]

From: Nikolay Aleksandrov <nikolay at cumulusnetworks.com>
Date: Mon, 18 Dec 2017 17:35:09 +0200

> The early call to br_stp_change_bridge_id in bridge's newlink can cause
> a memory leak if an error occurs during the newlink because the fdb
> entries are not cleaned up if a different lladdr was specified, also
> another minor issue is that it generates fdb notifications with
> ifindex = 0. Another unrelated memory leak is the bridge sysfs entries
> which get added on NETDEV_REGISTER event, but are not cleaned up in the
> newlink error path. To remove this special case the call to
> br_stp_change_bridge_id is done after netdev register and we cleanup the
> bridge on changelink error via br_dev_delete to plug all leaks.
> 
> This patch makes netlink bridge destruction on newlink error the same as
> dellink and ioctl del which is necessary since at that point we have a
> fully initialized bridge device.
> 
> To reproduce the issue:
> $ ip l add br0 address 00:11:22:33:44:55 type bridge group_fwd_mask 1
> RTNETLINK answers: Invalid argument
> 
> $ rmmod bridge
> [ 1822.142525] =============================================================================
> [ 1822.143640] BUG bridge_fdb_cache (Tainted: G           O    ): Objects remaining in bridge_fdb_cache on __kmem_cache_shutdown()
> [ 1822.144821] -----------------------------------------------------------------------------
 ...
> Fixes: 30313a3d5794 ("bridge: Handle IFLA_ADDRESS correctly when creating bridge device")
> Fixes: 5b8d5429daa0 ("bridge: netlink: register netdevice before executing changelink")
> Signed-off-by: Nikolay Aleksandrov <nikolay at cumulusnetworks.com>
> ---
> v2: it's better to use br_dev_delete to clean up because the sysfs entries
> were leaked as well, basically we use it for the bridge fdb flush and sysfs
> entries delete
> 
> Consequently this also would fix the null ptr deref due to the rhashtable
> not being initialized in net-next when br_stp_change_bridge_id is called.

Applied, but doing state changes after register_netdev() is always dicey.

I hope things go well if the device is brought up immediately during
that register_netdev() call and before the stp bridge ID setting is
done.