Return-Path: linux-nfs-owner@vger.kernel.org Received: from mail-qa0-f50.google.com ([209.85.216.50]:56235 "EHLO mail-qa0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932498AbaKMMay (ORCPT ); Thu, 13 Nov 2014 07:30:54 -0500 Received: by mail-qa0-f50.google.com with SMTP id w8so653093qac.23 for ; Thu, 13 Nov 2014 04:30:53 -0800 (PST) From: Jeff Layton To: trond.myklebust@primarydata.com Cc: bfields@fieldses.org, linux-nfs@vger.kernel.org Subject: [PATCH] sunrpc: fix sleeping under rcu_read_lock in gss_stringify_acceptor Date: Thu, 13 Nov 2014 07:30:46 -0500 Message-Id: <1415881846-11873-1-git-send-email-jlayton@primarydata.com> Sender: linux-nfs-owner@vger.kernel.org List-ID: Bruce reported that he was seeing the following BUG pop: BUG: sleeping function called from invalid context at mm/slab.c:2846 in_atomic(): 0, irqs_disabled(): 0, pid: 4539, name: mount.nfs 2 locks held by mount.nfs/4539: #0: (nfs_clid_init_mutex){+.+.+.}, at: [] nfs4_discover_server_trunking+0x4a/0x2f0 [nfsv4] #1: (rcu_read_lock){......}, at: [] gss_stringify_acceptor+0x5/0xb0 [auth_rpcgss] Preemption disabled at:[] printk+0x4d/0x4f CPU: 3 PID: 4539 Comm: mount.nfs Not tainted 3.18.0-rc1-00013-g5b095e9 #3393 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 ffff880021499390 ffff8800381476a8 ffffffff81a534cf 0000000000000001 0000000000000000 ffff8800381476c8 ffffffff81097854 00000000000000d0 0000000000000018 ffff880038147718 ffffffff8118e4f3 0000000020479f00 Call Trace: [] dump_stack+0x4f/0x7c [] __might_sleep+0x114/0x180 [] __kmalloc+0x1a3/0x280 [] gss_stringify_acceptor+0x58/0xb0 [auth_rpcgss] [] ? gss_stringify_acceptor+0x5/0xb0 [auth_rpcgss] [] rpcauth_stringify_acceptor+0x18/0x30 [sunrpc] [] nfs4_proc_setclientid+0x199/0x380 [nfsv4] [] ? nfs4_proc_setclientid+0x200/0x380 [nfsv4] [] nfs40_discover_server_trunking+0xda/0x150 [nfsv4] [] ? nfs40_discover_server_trunking+0x5/0x150 [nfsv4] [] nfs4_discover_server_trunking+0x7f/0x2f0 [nfsv4] [] nfs4_init_client+0x104/0x2f0 [nfsv4] [] nfs_get_client+0x314/0x3f0 [nfs] [] ? nfs_get_client+0xe0/0x3f0 [nfs] [] nfs4_set_client+0x8a/0x110 [nfsv4] [] ? __rpc_init_priority_wait_queue+0xa8/0xf0 [sunrpc] [] nfs4_create_server+0x12f/0x390 [nfsv4] [] nfs4_remote_mount+0x32/0x60 [nfsv4] [] mount_fs+0x39/0x1b0 [] ? __alloc_percpu+0x15/0x20 [] vfs_kern_mount+0x6b/0x150 [] nfs_do_root_mount+0x86/0xc0 [nfsv4] [] nfs4_try_mount+0x44/0xc0 [nfsv4] [] ? get_nfs_version+0x27/0x90 [nfs] [] nfs_fs_mount+0x47d/0xd60 [nfs] [] ? mutex_unlock+0xe/0x10 [] ? nfs_remount+0x430/0x430 [nfs] [] ? nfs_clone_super+0x140/0x140 [nfs] [] mount_fs+0x39/0x1b0 [] ? __alloc_percpu+0x15/0x20 [] vfs_kern_mount+0x6b/0x150 [] do_mount+0x210/0xbe0 [] ? copy_mount_options+0x3a/0x160 [] SyS_mount+0x6f/0xb0 [] system_call_fastpath+0x12/0x17 Sleeping under the rcu_read_lock is bad. This patch fixes it by dropping the rcu_read_lock before doing the allocation and then reacquiring it and redoing the dereference before doing the copy. If we find that the string has somehow grown in the meantime, we'll reallocate and try again. Cc: # v3.17+ Reported-by: "J. Bruce Fields" Signed-off-by: Jeff Layton --- net/sunrpc/auth_gss/auth_gss.c | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c index afb292cd797d..53ed8d3f8897 100644 --- a/net/sunrpc/auth_gss/auth_gss.c +++ b/net/sunrpc/auth_gss/auth_gss.c @@ -1353,6 +1353,7 @@ gss_stringify_acceptor(struct rpc_cred *cred) char *string = NULL; struct gss_cred *gss_cred = container_of(cred, struct gss_cred, gc_base); struct gss_cl_ctx *ctx; + unsigned int len; struct xdr_netobj *acceptor; rcu_read_lock(); @@ -1360,15 +1361,39 @@ gss_stringify_acceptor(struct rpc_cred *cred) if (!ctx) goto out; - acceptor = &ctx->gc_acceptor; + len = ctx->gc_acceptor.len; + rcu_read_unlock(); /* no point if there's no string */ - if (!acceptor->len) - goto out; - - string = kmalloc(acceptor->len + 1, GFP_KERNEL); + if (!len) + return NULL; +realloc: + string = kmalloc(len + 1, GFP_KERNEL); if (!string) + return NULL; + + rcu_read_lock(); + ctx = rcu_dereference(gss_cred->gc_ctx); + + /* did the ctx disappear or was it replaced by one with no acceptor? */ + if (!ctx || !ctx->gc_acceptor.len) { + kfree(string); + string = NULL; goto out; + } + + acceptor = &ctx->gc_acceptor; + + /* + * Did we find a new acceptor that's longer than the original? Allocate + * a longer buffer and try again. + */ + if (len < acceptor->len) { + len = acceptor->len; + rcu_read_unlock(); + kfree(string); + goto realloc; + } memcpy(string, acceptor->data, acceptor->len); string[acceptor->len] = '\0'; -- 2.1.0