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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

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.