netdev - Re: [B.A.T.M.A.N.] [PATCH net] net: batman-adv: Fix possible memleaks when fail to register

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

Date:   Wed, 26 Apr 2017 09:16:54 +0200
From:   Sven Eckelmann <sven@...fation.org>
To:     Gao Feng <gfree.wind@...mail.com>
Cc:     b.a.t.m.a.n@...ts.open-mesh.org, mareklindner@...mailbox.ch,
        sw@...onwunderlich.de, a@...table.cc, davem@...emloft.net,
        netdev@...r.kernel.org, 'Gao Feng' <fgao@...ai8.com>
Subject: Re: [B.A.T.M.A.N.] [PATCH net] net: batman-adv: Fix possible memleaks when fail to register_netdevice

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. 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.

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/netdev@vger.kernel.org/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

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)