Re: [PATCH iproute2] iproute2: fix use-after-free

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 12 Sep 2018 23:07:20 -0700
Mahesh Bandewar (महेश बंडेवार) <maheshb@google.com> wrote:

> On Wed, Sep 12, 2018 at 5:33 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Wed, 12 Sep 2018 16:29:28 -0700
> > Mahesh Bandewar <mahesh@bandewar.net> wrote:
> >  
> > > From: Mahesh Bandewar <maheshb@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 <maheshb@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.

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.