Re: [Patch net v2] genetlink: fix memory leaks in genl_family_rcv_msg_dumpit()

From: Cong Wang <xiyou.wangcong@gmail.com>
To: Ido Schimmel <idosch@idosch.org>
Cc: Linux Kernel Network Developers <netdev@vger.kernel.org>,
	syzbot+21f04f481f449c8db840@syzkaller.appspotmail.com,
	"Jason A. Donenfeld" <Jason@zx2c4.com>,
	Florian Westphal <fw@strlen.de>,
	Pablo Neira Ayuso <pablo@netfilter.org>,
	Jiri Pirko <jiri@mellanox.com>,
	YueHaibing <yuehaibing@huawei.com>,
	Shaochun Chen <cscnull@gmail.com>
Subject: Re: [Patch net v2] genetlink: fix memory leaks in genl_family_rcv_msg_dumpit()
Date: Wed, 10 Jun 2020 21:14:10 -0700	[thread overview]
Message-ID: <CAM_iQpVU14ky_L7PqhyB3_OSPpbY5ZKLn=GonLU88GcN5fEoHw@mail.gmail.com> (raw)
In-Reply-To: <20200610142700.GA2174714@splinter>

On Wed, Jun 10, 2020 at 7:27 AM Ido Schimmel <idosch@idosch.org> wrote:
> > @@ -548,7 +591,7 @@ static int genl_lock_done(struct netlink_callback *cb)
> >               rc = ops->done(cb);
> >               genl_unlock();
> >       }
> > -     genl_family_rcv_msg_attrs_free(info->family, info->attrs, true);
> > +     genl_family_rcv_msg_attrs_free(info->family, info->attrs, false);
>
> Cong,
>
> This seems to result in a memory leak because 'info->attrs' is never
> freed in the non-parallel case.
>
> Both the parallel and non-parallel code paths call genl_start() which
> allocates the array, but the latter calls genl_lock_done() as its done()
> callback which never frees it.

Good catch! Looks like I should just revert the above chunk. The
last parameter of genl_family_rcv_msg_attrs_free() is just confusing,
genl_lock_done() is clearly not parallel at all..

I will take a deeper look and send out a patch tomorrow.

Thanks!