[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAF2d9jg6-nZq9tXG6xg8Mu1A0-W+13Jj-YCaXQAAKxWg08rj0A@mail.gmail.com>
Date: Thu, 13 Sep 2018 10:54:42 -0700
From: Mahesh Bandewar (महेश बंडेवार)
<maheshb@...gle.com>
To: Stephen Hemminger <stephen@...workplumber.org>
Cc: Mahesh Bandewar <mahesh@...dewar.net>,
netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH iproute2] iproute2: fix use-after-free
On Thu, Sep 13, 2018 at 8:19 AM, Stephen Hemminger
<stephen@...workplumber.org> wrote:
>
> On Wed, 12 Sep 2018 23:07:20 -0700
> Mahesh Bandewar (महेश बंडेवार) <maheshb@...gle.com> wrote:
>
> > On Wed, Sep 12, 2018 at 5:33 PM, Stephen Hemminger
> > <stephen@...workplumber.org> wrote:
> > >
> > > On Wed, 12 Sep 2018 16:29:28 -0700
> > > Mahesh Bandewar <mahesh@...dewar.net> wrote:
> > >
> > > > From: Mahesh Bandewar <maheshb@...gle.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@...gle.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.
Powered by blists - more mailing lists