On Mon, 7 Oct 2019 at 20:22, Sabrina Dubroca <sd@xxxxxxxxxxxxxxx> wrote: > Hi Sabrina, Thank you for the review! > 2019-09-28, 16:48:43 +0000, Taehee Yoo wrote: > > virt_wifi_newlink() calls netdev_upper_dev_link() and it internally > > holds reference count of lower interface. > > > > Current code does not release a reference count of the lower interface > > when the lower interface is being deleted. > > So, reference count leaks occur. > > > > Test commands: > > ip link add dummy0 type dummy > > ip link add vw1 link dummy0 type virt_wifi > > There should also be "ip link del dummy0" in this reproducer, right? > > [...] > > > @@ -598,14 +634,24 @@ static int __init virt_wifi_init_module(void) > > /* Guaranteed to be locallly-administered and not multicast. */ > > eth_random_addr(fake_router_bssid); > > > > + err = register_netdevice_notifier(&virt_wifi_notifier); > > + if (err) > > + return err; > > + > > Here err is 0. > > > common_wiphy = virt_wifi_make_wiphy(); > > if (!common_wiphy) > > - return -ENOMEM; > > + goto notifier; > > err is still 0 when we jump... > > > err = rtnl_link_register(&virt_wifi_link_ops); > > if (err) > > - virt_wifi_destroy_wiphy(common_wiphy); > > + goto destroy_wiphy; > > > > + return 0; > > + > > +destroy_wiphy: > > + virt_wifi_destroy_wiphy(common_wiphy); > > +notifier: > > + unregister_netdevice_notifier(&virt_wifi_notifier); > > return err; > > } > > ... so now we return 0 on failure. Can you add an "err = -ENOMEM" > before "common_wiphy = ..."? > You're right, I will fix this in a v5 patch! Thanks! > Thanks. > > -- > Sabrina
