Roopa Prabhu <[email protected]> writes: > On Mon, Dec 17, 2018 at 6:58 AM Petr Machata <[email protected]> wrote: >> >> When a failure occurs in rtnl_configure_link(), the current code >> calls unregister_netdevice() to roll back the earlier call to >> register_netdevice(), and jumps to errout, which calls >> vxlan_fdb_destroy(). >> >> However unregister_netdevice() calls transitively ndo_uninit(), >> which is vxlan_uninit(), and that already takes care of deleting >> the default FDB entry by calling vxlan_fdb_delete_default(). >> Since the entry added earlier in __vxlan_dev_create() is exactly >> the default entry, the cleanup code in the errout block always >> leads to double free and thus a panic. >> >> Fix by skipping the rollback code and just returning. > > I trust your test case that pointed to a potential issue..., but can > we add a comment there on > why there is an fdb add but no destroy in the roll back code ?. Also,
This is at a place in the code where the clean-up would normally be: >> + /* the all_zeros_mac entry was deleted at vxlan_uninit */ Is that what you meant? > the destroy code from uninit path does > look for an entry being present before attempting a destroy, its > unclear to me yet why that ends up doing a double free ? That code is called first, finds the FDB entry, destroys it. After it returns, another destroy call is invoked from __vxlan_dev_create(). >> @@ -3298,8 +3299,8 @@ static int __vxlan_dev_create(struct net *net, struct >> net_device *dev, >> list_add(&vxlan->next, &vn->vxlan_list); >> return 0; >> errout: >> - if (f) >> - vxlan_fdb_destroy(vxlan, f, false); >> + unregister_netdevice(dev); >> + /* the all_zeros_mac entry was deleted at vxlan_uninit */ >> return err; >> } >> >> -- >> 2.4.11 >>
