linux-kernel - Re: net: GPF in __netlink_ns

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: <87vb6q63co.fsf@x220.int.ebiederm.org>
Date:	Mon, 18 Jan 2016 14:26:47 -0600
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Herbert Xu <herbert@...dor.apana.org.au>
Cc:	Richard Weinberger <richard.weinberger@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Thomas Graf <tgraf@...g.ch>,
	Daniel Borkmann <daniel@...earbox.net>,
	Ken-ichirou MATSUZAWA <chamaken@...il.com>,
	Nicolas Dichtel <nicolas.dichtel@...nd.com>,
	Florian Westphal <fw@...len.de>,
	netdev <netdev@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	syzkaller <syzkaller@...glegroups.com>,
	Kostya Serebryany <kcc@...gle.com>,
	Alexander Potapenko <glider@...gle.com>,
	Sasha Levin <sasha.levin@...cle.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Dmitry Vyukov <dvyukov@...gle.com>,
	Kaike Wan <kaike.wan@...el.com>,
	John Fleck <john.fleck@...el.com>,
	Ira Weiny <ira.weiny@...el.com>,
	Doug Ledford <dledford@...hat.com>
Subject: Re: net: GPF in __netlink_ns_capable


Apparently we have missed entirely the folks who added this chunk
of this code to the kernel on this thread, so adding them now.

ebiederm@...ssion.com (Eric W. Biederman) writes:

> Herbert Xu <herbert@...dor.apana.org.au> writes:
>
>> On Sat, Jan 16, 2016 at 01:08:33AM +0100, Richard Weinberger wrote:
>>> On Fri, Jan 15, 2016 at 11:31 PM, Dmitry Vyukov <dvyukov@...gle.com> wrote:
>>> > Call Trace:
>>> >  [<     inline     >] netlink_ns_capable net/netlink/af_netlink.c:1417
>>> >  [<ffffffff8529c0a5>] netlink_capable+0x25/0x30 net/netlink/af_netlink.c:1432
>>> 
>>> Hmm, we're crashing because NETLINK_CB(skb).sk is NULL.
>>> netlink_dump() creates a new skb without a netlink control block,
>>> but infiniband's dump functions use netlink_capable() which needs a valid
>>> NETLINK_CB(skb).sk.
>>> 
>>> What about something like that?
>>
>> No you can't do it here as netlink_unicast also calls this and for
>> that case you'd be overwriting the original sending user-space
>> socket with the kernel socket.
>>
>> I'm adding Eric Bierderman as he wrote some of the code in question.
>
> *Scratches my head*
>
> I think I am just going to recommend removing that bit of code from
> the infiniband stack.
>
> There are several things very wrong here.
>
> First netlinnk_capable and it's ilk are for the very specific purpose of
> handling backwards compatibility as a truly clean solution of checking
> at open or connect time would break existing applications.
> ib_nl_handle_resolv_resp is new code.  So it can almost certainly do
> something cleaner.
>
> netlink_capable is very much not for filtering netlink dumps, but for
> filtering the queries themselves.  So it appears the capability check is
> very much in the wrong place.
>
> All of this is newish code and apparently never even tested as this code
> should have failed this way for everyone.  So since the code does not
> work not apparently has never worked, it is probably easiest just to
> remove the problematic code (AKA revert), and start fresh and not
> something that requires backwards compatibility hacks from day one.
>
> By new I mean code that came in through the commit below.
>
> commit 2ca546b92a024d07adedd15b4c262b1c2c0786ec
> Author: Kaike Wan <kaike.wan@...el.com>
> Date:   Fri Aug 14 08:52:09 2015 -0400
>
>     IB/sa: Route SA pathrecord query through netlink
>     
>     This patch routes a SA pathrecord query to netlink first and processes the
>     response appropriately. If a failure is returned, the request will be sent
>     through IB. The decision whether to route the request to netlink first is
>     determined by the presence of a listener for the local service netlink
>     multicast group. If the user-space local service netlink multicast group
>     listener is not present, the request will be sent through IB, just like
>     what is currently being done.
>     
>     Signed-off-by: Kaike Wan <kaike.wan@...el.com>
>     Signed-off-by: John Fleck <john.fleck@...el.com>
>     Signed-off-by: Ira Weiny <ira.weiny@...el.com>
>     Signed-off-by: Doug Ledford <dledford@...hat.com>

What was this code trying to do with netlink_capable besides oops the
kernel?

Eric