From patchwork Sun Feb 16 17:27:33 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Trond Myklebust X-Patchwork-Id: 3658291 Return-Path: X-Original-To: patchwork-linux-nfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id EBEFC9F1EE for ; Sun, 16 Feb 2014 17:27:59 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 014FF201BC for ; Sun, 16 Feb 2014 17:27:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id BAAD62017A for ; Sun, 16 Feb 2014 17:27:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753200AbaBPR1j (ORCPT ); Sun, 16 Feb 2014 12:27:39 -0500 Received: from mail-ob0-f169.google.com ([209.85.214.169]:51293 "EHLO mail-ob0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752796AbaBPR1h (ORCPT ); Sun, 16 Feb 2014 12:27:37 -0500 Received: by mail-ob0-f169.google.com with SMTP id wo20so16132253obc.28 for ; Sun, 16 Feb 2014 09:27:36 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:organization:content-type:mime-version :content-transfer-encoding; bh=OxtGg0Jm1rSVEGvA6m+rSEOWXeQj2w8gRdQ3zh4SJhI=; b=a5NGynnOMh2d9x+CYXmnj+swuL8oyDCfz0atiA6ELoijaGXQQ1bRnBHUdObKijYVNT zjt++BI0gSzbwuC8wuFz438628CbDEIaugnAcc12V78YPfXzR5Mw1Dd3GfEPsGtwZ6tN Cvb7DMBqC8Y8bOXQStU70KZmpfzlwDqry+Wjls44Yt4U9c9npFiNUMS0wjsgrwNMFG06 mwfKM0sx3z2rYhSDl2AUxMMHzZQFKzgkeZksAzt0BDxpFKTujaAEEqdd+pEdkI3cU8JH gxAevZOz7hKKK/wqVx3wtm65EvG8/Rqr74CWgthcdodHeVLT8TtCie4qlDlryjw81rgL qn7Q== X-Gm-Message-State: ALoCoQmGhCDddVnZBdWXmVedfBKjaivsLOfjkkBevl2ImqSMVAFjkyK1vOm7tGALBmg3IWWy8GGh X-Received: by 10.182.225.5 with SMTP id rg5mr1182058obc.58.1392571656468; Sun, 16 Feb 2014 09:27:36 -0800 (PST) Received: from [172.16.74.174] (c-98-209-19-95.hsd1.mi.comcast.net. [98.209.19.95]) by mx.google.com with ESMTPSA id wj7sm38548655obc.8.2014.02.16.09.27.34 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Sun, 16 Feb 2014 09:27:35 -0800 (PST) Message-ID: <1392571653.44773.4.camel@leira.trondhjem.org> Subject: Re: [BUG] unable to handle kernel NULL pointer dereference From: Trond Myklebust To: Borislav Petkov , Linux NFS Mailing List Cc: John , lkml , "netdev@vger.kernel.org" , "stephen@networkplumber.org" , "mlindner@marvell.com" , "J. Bruce Fields" Date: Sun, 16 Feb 2014 12:27:33 -0500 In-Reply-To: <20140215232508.GB4508@pd.tnic> References: <1392466251.41282.YahooMailNeo@web140003.mail.bf1.yahoo.com> <1392494917.71728.YahooMailNeo@web140002.mail.bf1.yahoo.com> <20140215203015.GA4528@pd.tnic> <1392498262.98385.YahooMailNeo@web140003.mail.bf1.yahoo.com> <20140215232508.GB4508@pd.tnic> Organization: PrimaryData Inc X-Mailer: Evolution 3.10.3 (3.10.3-1.fc20) Mime-Version: 1.0 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Please ensure that you post to the linux-nfs@vger.kernel.org when reporting NFS and RPC related bugs. On Sun, 2014-02-16 at 00:25 +0100, Borislav Petkov wrote: > On Sat, Feb 15, 2014 at 01:04:22PM -0800, John wrote: > > Thanks for the reply, Boris. The .config is unmodified > > from the Arch Distro default for 3.13.3-1 which can be found > > here: http://pastebin.com/LPGZ8ZqA > > Yep, it is that struct net *net argument to put_pipe_version() which is NULL: > > 12: 55 push %ebp > 13: 89 e5 mov %esp,%ebp > 15: 56 push %esi > 16: 53 push %ebx > 17: 3e 8d 74 26 00 lea %ds:0x0(%esi,%eiz,1),%esi > 1c: 8b 1d 28 e9 a3 f8 mov 0xf8a3e928,%ebx > 22: 89 c6 mov %eax,%esi > 24: e8 59 64 5f c8 call 0xc85f6482 > 29: 85 db test %ebx,%ebx > 2b:* 8b 86 58 08 00 00 mov 0x858(%esi),%eax <-- trapping instruction > > put_pipe_version: > pushl %ebp # > movl %esp, %ebp #, > pushl %esi # > pushl %ebx # > call mcount > movl sunrpc_net_id, %ebx # sunrpc_net_id, sunrpc_net_id.130 > movl %eax, %esi # net, net > call __rcu_read_lock # > testl %ebx, %ebx # sunrpc_net_id.130 > movl 2136(%esi), %eax # MEM[(struct net_generic * const *)net_4(D) + 2136B], ng <-- trapping insn > > > [ 137.689996] ESI: 00000000 EDI: f56efc00 EBP: f568fee8 ESP: f568fee0 > ^^^^^^^^ > > Here's the c/asm interleaved version: > > static void put_pipe_version(struct net *net) > { > d80: 55 push %ebp > d81: 89 e5 mov %esp,%ebp > d83: 56 push %esi > d84: 53 push %ebx > d85: e8 fc ff ff ff call d86 > d86: R_386_PC32 mcount > struct sunrpc_net *sn = net_generic(net, sunrpc_net_id); > d8a: 8b 1d 00 00 00 00 mov 0x0,%ebx > d8c: R_386_32 sunrpc_net_id > spin_unlock(&pipe_version_lock); > return ret; > } > > static void put_pipe_version(struct net *net) > { > d90: 89 c6 mov %eax,%esi > * block, but only when acquiring spinlocks that are subject to priority > * inheritance. > */ > static inline void rcu_read_lock(void) > { > __rcu_read_lock(); > d92: e8 fc ff ff ff call d93 > d93: R_386_PC32 __rcu_read_lock > struct net_generic *ng; > void *ptr; > > rcu_read_lock(); > ng = rcu_dereference(net->gen); > BUG_ON(id == 0 || id > ng->len); > d97: 85 db test %ebx,%ebx > { > struct net_generic *ng; > void *ptr; > > rcu_read_lock(); > ng = rcu_dereference(net->gen); > d99: 8b 86 58 08 00 00 mov 0x858(%esi),%eax <-- trapping insn > > > I guess you could avoid the crash if you did > > if (!net) > return; > > in put_pipe_version() but this hardly is the right solution. Someone > else has to make sense of this thing, not me. :-) Does the following patch help? 8<------------------------------------------------------------------- From 0e57b109cd7b17d6e6f16c3454427372a583b18a Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Sun, 16 Feb 2014 12:14:13 -0500 Subject: [PATCH] SUNRPC: Ensure that gss_auth isn't freed before its upcall messages Fix a race in which the RPC client is shutting down while the gss daemon is processing a downcall. If the RPC client manages to shut down before the gss daemon is done, then the struct gss_auth used in gss_release_msg() may have already been freed. Link: http://lkml.kernel.org/r/1392494917.71728.YahooMailNeo@web140002.mail.bf1.yahoo.com Reported-by: John Reported-by: Borislav Petkov Signed-off-by: Trond Myklebust Tested-by: John Tested-by: John --- net/sunrpc/auth_gss/auth_gss.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c index 44a61e8fda6f..1ba1fd114912 100644 --- a/net/sunrpc/auth_gss/auth_gss.c +++ b/net/sunrpc/auth_gss/auth_gss.c @@ -108,6 +108,7 @@ struct gss_auth { static DEFINE_SPINLOCK(pipe_version_lock); static struct rpc_wait_queue pipe_version_rpc_waitqueue; static DECLARE_WAIT_QUEUE_HEAD(pipe_version_waitqueue); +static void gss_put_auth(struct gss_auth *gss_auth); static void gss_free_ctx(struct gss_cl_ctx *); static const struct rpc_pipe_ops gss_upcall_ops_v0; @@ -320,6 +321,7 @@ gss_release_msg(struct gss_upcall_msg *gss_msg) if (gss_msg->ctx != NULL) gss_put_ctx(gss_msg->ctx); rpc_destroy_wait_queue(&gss_msg->rpc_waitqueue); + gss_put_auth(gss_msg->auth); kfree(gss_msg); } @@ -500,6 +502,7 @@ gss_alloc_msg(struct gss_auth *gss_auth, if (err) goto err_free_msg; }; + kref_get(&gss_auth->kref); return gss_msg; err_free_msg: kfree(gss_msg); @@ -1064,6 +1067,12 @@ gss_free_callback(struct kref *kref) } static void +gss_put_auth(struct gss_auth *gss_auth) +{ + kref_put(&gss_auth->kref, gss_free_callback); +} + +static void gss_destroy(struct rpc_auth *auth) { struct gss_auth *gss_auth = container_of(auth, @@ -1084,7 +1093,7 @@ gss_destroy(struct rpc_auth *auth) gss_auth->gss_pipe[1] = NULL; rpcauth_destroy_credcache(auth); - kref_put(&gss_auth->kref, gss_free_callback); + gss_put_auth(gss_auth); } /* @@ -1255,7 +1264,7 @@ gss_destroy_nullcred(struct rpc_cred *cred) call_rcu(&cred->cr_rcu, gss_free_cred_callback); if (ctx) gss_put_ctx(ctx); - kref_put(&gss_auth->kref, gss_free_callback); + gss_put_auth(gss_auth); } static void