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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180913081948.1bf8368b@xeon-e3>
Date:   Thu, 13 Sep 2018 08:19:48 -0700
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     "Mahesh Bandewar (महेश बंडेवार)" <maheshb@...gle.com>
Cc:     Mahesh Bandewar <mahesh@...dewar.net>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH iproute2] iproute2: fix use-after-free

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.

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.