Subject: Re: [PATCH net] net: batman-adv: Fix possible memleaks when fail to register_netdevice

From: "Gao Feng" <[email-masked]>
Date: Wed, 26 Apr 2017 15:28:26 +0800

> From: Sven Eckelmann [mailto:[email-masked]]
> Sent: Wednesday, April 26, 2017 3:17 PM
> On Mittwoch, 26. April 2017 14:44:24 CEST Gao Feng wrote:
> [...]
> > I get it now, thanks.
> [...]
> > BTW, I think although the batadv_softif_create is legacy, we should
> > fix it when it still exists :)
> 
> I didn't meant that we should not fix it. I just said that it looks to me
like the fix
> should look different to ensure that it actually fixes the sysfs and rtnl
link
> implementation for the batadv interface creation. Right now the ndo_uninit
> (when it would be set by batadv) is called in the netdev core functions
when an
> error happens during the registration. This is not the case for the
destructor.

Thanks your answer.
I assumed the destructor is not for this case before.. 

> Your patch would not change it. It therefore looks like you simply have to
move
> the current destructor (without the free_netdev) to ndo_uninit and change
the
> destructor to free_netdev.

Yes, that patch didn't touch badman-adv. Because current badman-adv doesn't
support newlink now.
It would be good that cleanup the resource in ndo_uninit routine.

Best Regards
Feng
 
> 
> The batadv ops doesn't have a newlink function. It will therefore use the
> register_netdevice code path which calls free_netdev on failures. The
extra
> cleanup you've added in
> https://www.mail-archive.com/[email-masked]/msg165253.html can
> therefore not work for batman-adv. Actually, it is not touching anything
> batman-adv related. The suggestion to change the register_netdevice ->
> free_netdev part in rtnl_newlink was new in the reply to the batadv
discussion.
> It is therefore still an open discussion how it is correctly fixed.
> 
> Kind regards,
> 	Sven