From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756658AbcARUgD (ORCPT ); Mon, 18 Jan 2016 15:36:03 -0500 Received: from out02.mta.xmission.com ([166.70.13.232]:46588 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756071AbcARUf5 (ORCPT ); Mon, 18 Jan 2016 15:35:57 -0500 From: ebiederm@xmission.com (Eric W. Biederman) To: Herbert Xu Cc: Richard Weinberger , "David S. Miller" , Thomas Graf , Daniel Borkmann , Ken-ichirou MATSUZAWA , Nicolas Dichtel , Florian Westphal , netdev , LKML , syzkaller , Kostya Serebryany , Alexander Potapenko , Sasha Levin , Eric Dumazet , Dmitry Vyukov , Kaike Wan , John Fleck , Ira Weiny , Doug Ledford References: <20160118092047.GA30682@gondor.apana.org.au> <878u3m7i5q.fsf@x220.int.ebiederm.org> Date: Mon, 18 Jan 2016 14:26:47 -0600 In-Reply-To: <878u3m7i5q.fsf@x220.int.ebiederm.org> (Eric W. Biederman's message of "Mon, 18 Jan 2016 14:21:37 -0600") Message-ID: <87vb6q63co.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-AID: U2FsdGVkX1/iijQrVFIYiZkEp3RSB8Wr8jaygCw+N1A= X-SA-Exim-Connect-IP: 97.121.81.63 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa01 1397; Body=1 Fuz1=1 Fuz2=1] X-Spam-DCC: XMission; sa01 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Herbert Xu X-Spam-Relay-Country: X-Spam-Timing: total 1211 ms - load_scoreonly_sql: 0.06 (0.0%), signal_user_changed: 3.7 (0.3%), b_tie_ro: 2.6 (0.2%), parse: 1.34 (0.1%), extract_message_metadata: 36 (3.0%), get_uri_detail_list: 4.8 (0.4%), tests_pri_-1000: 17 (1.4%), tests_pri_-950: 2.00 (0.2%), tests_pri_-900: 1.71 (0.1%), tests_pri_-400: 57 (4.7%), check_bayes: 51 (4.2%), b_tokenize: 20 (1.7%), b_tok_get_all: 12 (1.0%), b_comp_prob: 6 (0.5%), b_tok_touch_all: 6 (0.5%), b_finish: 0.93 (0.1%), tests_pri_0: 1078 (89.0%), check_dkim_signature: 0.84 (0.1%), check_dkim_adsp: 28 (2.3%), tests_pri_500: 9 (0.8%), poll_dns_idle: 0.23 (0.0%), rewrite_mail: 0.00 (0.0%) Subject: Re: net: GPF in __netlink_ns_capable X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Wed, 24 Sep 2014 11:00:52 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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@xmission.com (Eric W. Biederman) writes: > Herbert Xu 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 wrote: >>> > Call Trace: >>> > [< inline >] netlink_ns_capable net/netlink/af_netlink.c:1417 >>> > [] 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 > 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 > Signed-off-by: John Fleck > Signed-off-by: Ira Weiny > Signed-off-by: Doug Ledford What was this code trying to do with netlink_capable besides oops the kernel? Eric