On Thu, Sep 13, 2018 at 8:19 AM, Stephen Hemminger
<step...@networkplumber.org> wrote:
>
> On Wed, 12 Sep 2018 23:07:20 -0700
> Mahesh Bandewar (महेश बंडेवार) <mahe...@google.com> wrote:
>
> > On Wed, Sep 12, 2018 at 5:33 PM, Stephen Hemminger
> > <step...@networkplumber.org> wrote:
> > >
> > > On Wed, 12 Sep 2018 16:29:28 -0700
> > > Mahesh Bandewar <mah...@bandewar.net> wrote:
> > >
> > > > From: Mahesh Bandewar <mahe...@google.com>
> > > >
> > > > A local program using iproute2 lib pointed out the issue and looking
> > > > at the code it is pretty obvious -
> > > >
> > > > a = (struct nlmsghdr *)b;
> > > > ...
> > > > free(b);
> > > > if (a->nlmsg_seq == seq)
> > > > ...
> > > >
> > > > Fixes: 86bf43c7c2fd ("lib/libnetlink: update rtnl_talk to support
> > > > malloc buff at run time")
> > > > Signed-off-by: Mahesh Bandewar <mahe...@google.com>
> > >
> > > Yes, this is a real problem.
> > >
> > > Maybe a minimal patch like this would be enough:
> > actually that will leave the memory leak at the 'goto next' line (just
> > few lines below) since that jump will overwrite the buf.
>
> It looks like everytime in the while loop. a new buffer is allocated.
> So yes, it looks like old, my patch, and your patch would leak there
> was an error followed by other data in response.
> Though I doubt kernel would ever do that.
>
I started fixing the issue that I reported and then found-out the
memory leak and hence the first attempt of simple fix went into fixing
free-after-use as well as memory leak (in my patch). I'm not going to
claim that I know how and where this gets used, but my attempt was to
simply fix those two issues. I don't mind which fix you apply as long
as these issues get addressed.
> The only user of iov style messages to the kernel is in tc batching.
> My gut feeling is that if one message in batch has error, then
> the netlink code should return that error and stop processing more.
