All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Gao Feng" <gfree.wind@foxmail.com>
To: 'Sven Eckelmann' <sven@narfation.org>
Cc: mareklindner@neomailbox.ch, netdev@vger.kernel.org,
	b.a.t.m.a.n@lists.open-mesh.org, a@unstable.cc,
	'Gao Feng' <fgao@ikuai8.com>,
	davem@davemloft.net
Subject: Re: [B.A.T.M.A.N.] [PATCH net] net: batman-adv: Fix possible memleaks when fail to register_netdevice
Date: Wed, 26 Apr 2017 15:28:26 +0800	[thread overview]
Message-ID: <005501d2be5e$b2bb0550$18310ff0$@foxmail.com> (raw)
In-Reply-To: <2857108.moq831zFsU@bentobox>

> From: Sven Eckelmann [mailto:sven@narfation.org]
> 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/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



WARNING: multiple messages have this Message-ID (diff)
From: "Gao Feng" <gfree.wind-H32Fclmsjq1BDgjK7y7TUQ@public.gmane.org>
To: "'Sven Eckelmann'" <sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org>
Cc: mareklindner-rVWd3aGhH2z5bpWLKbzFeg@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r@public.gmane.org,
	a@unstable.cc,
	'Gao Feng' <fgao-KlmEoCYek3zQT0dZR+AlfA@public.gmane.org>,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
Subject: Re: [PATCH net] net: batman-adv: Fix possible memleaks when fail to register_netdevice
Date: Wed, 26 Apr 2017 15:28:26 +0800	[thread overview]
Message-ID: <005501d2be5e$b2bb0550$18310ff0$@foxmail.com> (raw)
In-Reply-To: <2857108.moq831zFsU@bentobox>

> From: Sven Eckelmann [mailto:sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org]
> 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/netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.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

  reply	other threads:[~2017-04-26  7:28 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-25 12:03 [B.A.T.M.A.N.] [PATCH net] net: batman-adv: Fix possible memleaks when fail to register_netdevice gfree.wind
2017-04-25 12:03 ` gfree.wind
2017-04-25 13:53 ` [B.A.T.M.A.N.] " Sven Eckelmann
2017-04-25 13:53   ` Sven Eckelmann
2017-04-26  0:41   ` [B.A.T.M.A.N.] " Gao Feng
2017-04-26  0:41     ` Gao Feng
2017-04-26  5:57     ` [B.A.T.M.A.N.] " Sven Eckelmann
2017-04-26  5:57       ` Sven Eckelmann
2017-04-26  6:44       ` [B.A.T.M.A.N.] " Gao Feng
2017-04-26  6:44         ` Gao Feng
2017-04-26  7:16         ` [B.A.T.M.A.N.] " Sven Eckelmann
2017-04-26  7:16           ` Sven Eckelmann
2017-04-26  7:28           ` Gao Feng [this message]
2017-04-26  7:28             ` Gao Feng
2017-06-09  7:23 ` [B.A.T.M.A.N.] " Sven Eckelmann
2017-06-09  7:26   ` 高峰
2017-06-09  7:26     ` 高峰

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='005501d2be5e$b2bb0550$18310ff0$@foxmail.com' \
    --to=gfree.wind@foxmail.com \
    --cc=a@unstable.cc \
    --cc=b.a.t.m.a.n@lists.open-mesh.org \
    --cc=davem@davemloft.net \
    --cc=fgao@ikuai8.com \
    --cc=mareklindner@neomailbox.ch \
    --cc=netdev@vger.kernel.org \
    --cc=sven@narfation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.