From 168e4b39d1afb79a7e3ea6c3bb246b4c82c6bdb9 Mon Sep 17 00:00:00 2001 From: Weston Andros Adamson Date: Tue, 30 Oct 2012 17:01:40 -0400 Subject: SUNRPC: add WARN_ON_ONCE for potential deadlock rpc_shutdown_client should never be called from a workqueue context. If it is, it could deadlock looping forever trying to kill tasks that are assigned to the same kworker thread (and will never run rpc_exit_task). Signed-off-by: Weston Andros Adamson Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index cdc7564b4512..dd2532c10324 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -607,6 +607,13 @@ EXPORT_SYMBOL_GPL(rpc_killall_tasks); */ void rpc_shutdown_client(struct rpc_clnt *clnt) { + /* + * To avoid deadlock, never call rpc_shutdown_client from a + * workqueue context! + */ + WARN_ON_ONCE(current->flags & PF_WQ_WORKER); + might_sleep(); + dprintk_rcu("RPC: shutting down %s client for %s\n", clnt->cl_protname, rcu_dereference(clnt->cl_xprt)->servername); -- cgit v1.2.1 From 922eeac30d8456b8e4462cfb94ddbb6846790ad4 Mon Sep 17 00:00:00 2001 From: Weston Andros Adamson Date: Tue, 23 Oct 2012 10:43:25 -0400 Subject: SUNRPC: remove BUG_ON in __rpc_clnt_handle_event Print a KERN_INFO message before rpc_d_lookup_sb returns NULL, like other error paths in that function. Signed-off-by: Weston Andros Adamson Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index dd2532c10324..245de1a208f4 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -132,8 +132,10 @@ static struct dentry *rpc_setup_pipedir_sb(struct super_block *sb, int error; dir = rpc_d_lookup_sb(sb, dir_name); - if (dir == NULL) + if (dir == NULL) { + pr_info("RPC: pipefs directory doesn't exist: %s\n", dir_name); return dir; + } for (;;) { q.len = snprintf(name, sizeof(name), "clnt%x", (unsigned int)clntid++); name[sizeof(name) - 1] = '\0'; @@ -192,7 +194,8 @@ static int __rpc_clnt_handle_event(struct rpc_clnt *clnt, unsigned long event, case RPC_PIPEFS_MOUNT: dentry = rpc_setup_pipedir_sb(sb, clnt, clnt->cl_program->pipe_dir_name); - BUG_ON(dentry == NULL); + if (!dentry) + return -ENOENT; if (IS_ERR(dentry)) return PTR_ERR(dentry); clnt->cl_dentry = dentry; -- cgit v1.2.1 From 9a6478f6cccbd15af30f02368e68b47390360485 Mon Sep 17 00:00:00 2001 From: Weston Andros Adamson Date: Tue, 23 Oct 2012 10:43:27 -0400 Subject: SUNRPC: remove BUG_ON from rpc_run_bc_task Replace BUG_ON() with WARN_ON_ONCE() - rpc_run_bc_task calls rpc_init_task() then increments the tk_count, so this is a simple sanity check that if hit once would hit every time this code path is executed. Signed-off-by: Weston Andros Adamson Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 245de1a208f4..32aea0b779c5 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -918,7 +918,7 @@ struct rpc_task *rpc_run_bc_task(struct rpc_rqst *req, task->tk_action = call_bc_transmit; atomic_inc(&task->tk_count); - BUG_ON(atomic_read(&task->tk_count) != 2); + WARN_ON_ONCE(atomic_read(&task->tk_count) != 2); rpc_execute(task); out: -- cgit v1.2.1 From 576e613d2111036a075b7c4f7cafdf29a0021c29 Mon Sep 17 00:00:00 2001 From: Weston Andros Adamson Date: Tue, 23 Oct 2012 10:43:28 -0400 Subject: SUNRPC: remove BUG_ON from call_transmit Remove unneeded BUG_ON() Signed-off-by: Weston Andros Adamson Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 1 - 1 file changed, 1 deletion(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 32aea0b779c5..f1ab4a8ae22c 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -1664,7 +1664,6 @@ call_transmit(struct rpc_task *task) task->tk_action = call_transmit_status; /* Encode here so that rpcsec_gss can use correct sequence number. */ if (rpc_task_need_encode(task)) { - BUG_ON(task->tk_rqstp->rq_bytes_sent != 0); rpc_xdr_encode(task); /* Did the encode result in an error condition? */ if (task->tk_status != 0) { -- cgit v1.2.1 From 1facf4c4a486d515759edc0b2ece927942dd8167 Mon Sep 17 00:00:00 2001 From: Weston Andros Adamson Date: Tue, 23 Oct 2012 10:43:30 -0400 Subject: SUNRPC: remove BUG_ON from call_bc_transmit Replace BUG_ON() with WARN_ON_ONCE(). Signed-off-by: Weston Andros Adamson Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index f1ab4a8ae22c..913140173739 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -1794,7 +1794,7 @@ call_bc_transmit(struct rpc_task *task) * We were unable to reply and will have to drop the * request. The server should reconnect and retransmit. */ - BUG_ON(task->tk_status == -EAGAIN); + WARN_ON_ONCE(task->tk_status == -EAGAIN); printk(KERN_NOTICE "RPC: Could not send backchannel reply " "error: %d\n", task->tk_status); break; -- cgit v1.2.1 From 8b827e1f1e46c45a4c9c389676f622e569d8a8ea Mon Sep 17 00:00:00 2001 From: Weston Andros Adamson Date: Tue, 23 Oct 2012 10:43:31 -0400 Subject: SUNRPC: remove BUG_ON from call_bc_transmit Remove redundant BUG_ON(). Signed-off-by: Weston Andros Adamson Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 1 - 1 file changed, 1 deletion(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 913140173739..a9dd1e835f70 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -1747,7 +1747,6 @@ call_bc_transmit(struct rpc_task *task) { struct rpc_rqst *req = task->tk_rqstp; - BUG_ON(task->tk_status != 0); task->tk_status = xprt_prepare_transmit(task); if (task->tk_status == -EAGAIN) { /* -- cgit v1.2.1 From 50d2bdb19734f9e9f21e63881a9b6c8db4cc0eb7 Mon Sep 17 00:00:00 2001 From: Weston Andros Adamson Date: Thu, 1 Nov 2012 16:04:40 -0400 Subject: SUNRPC: remove BUG_ON from rpc_call_sync Use WARN_ON_ONCE instead of calling BUG_ON and return -EINVAL when RPC_TASK_ASYNC flag is passed to rpc_call_sync. Signed-off-by: Weston Andros Adamson Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index a9dd1e835f70..50bc9db8762c 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -842,7 +842,12 @@ int rpc_call_sync(struct rpc_clnt *clnt, const struct rpc_message *msg, int flag }; int status; - BUG_ON(flags & RPC_TASK_ASYNC); + WARN_ON_ONCE(flags & RPC_TASK_ASYNC); + if (flags & RPC_TASK_ASYNC) { + rpc_release_calldata(task_setup_data.callback_ops, + task_setup_data.callback_data); + return -EINVAL; + } task = rpc_run_task(&task_setup_data); if (IS_ERR(task)) -- cgit v1.2.1 From f994c43d19a9116727d4c228d3f13db595bff562 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Thu, 1 Nov 2012 12:14:14 -0400 Subject: SUNRPC: Clean up rpc_bind_new_program We can and should use the rpc_create_args and __rpc_clone_client() to change the program and version number on the resulting rpc_client. Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 50bc9db8762c..c69e199b1082 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -703,21 +703,19 @@ struct rpc_clnt *rpc_bind_new_program(struct rpc_clnt *old, const struct rpc_program *program, u32 vers) { + struct rpc_create_args args = { + .program = program, + .prognumber = program->number, + .version = vers, + .authflavor = old->cl_auth->au_flavor, + .client_name = old->cl_principal, + }; struct rpc_clnt *clnt; - const struct rpc_version *version; int err; - BUG_ON(vers >= program->nrvers || !program->version[vers]); - version = program->version[vers]; - clnt = rpc_clone_client(old); + clnt = __rpc_clone_client(&args, old); if (IS_ERR(clnt)) goto out; - clnt->cl_procinfo = version->procs; - clnt->cl_maxproc = version->nrprocs; - clnt->cl_protname = program->name; - clnt->cl_prog = program->number; - clnt->cl_vers = version->number; - clnt->cl_stats = program->stats; err = rpc_ping(clnt); if (err != 0) { rpc_shutdown_client(clnt); -- cgit v1.2.1 From eb96d5c97b0825d542e9c4ba5e0a22b519355166 Mon Sep 17 00:00:00 2001 From: Andy Adamson Date: Tue, 27 Nov 2012 10:34:19 -0500 Subject: SUNRPC handle EKEYEXPIRED in call_refreshresult Currently, when an RPCSEC_GSS context has expired or is non-existent and the users (Kerberos) credentials have also expired or are non-existent, the client receives the -EKEYEXPIRED error and tries to refresh the context forever. If an application is performing I/O, or other work against the share, the application hangs, and the user is not prompted to refresh/establish their credentials. This can result in a denial of service for other users. Users are expected to manage their Kerberos credential lifetimes to mitigate this issue. Move the -EKEYEXPIRED handling into the RPC layer. Try tk_cred_retry number of times to refresh the gss_context, and then return -EACCES to the application. Signed-off-by: Andy Adamson Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index c69e199b1082..55e174f2d02f 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -1381,6 +1381,7 @@ call_refreshresult(struct rpc_task *task) return; case -ETIMEDOUT: rpc_delay(task, 3*HZ); + case -EKEYEXPIRED: case -EAGAIN: status = -EACCES; if (!task->tk_cred_retry) -- cgit v1.2.1 From cd6c5968582a273561464fe6b1e8cc8214be02df Mon Sep 17 00:00:00 2001 From: Stanislav Kinsbursky Date: Mon, 17 Dec 2012 20:18:52 +0300 Subject: SUNRPC: continue run over clients list on PipeFS event instead of break There are SUNRPC clients, which program doesn't have pipe_dir_name. These clients can be skipped on PipeFS events, because nothing have to be created or destroyed. But instead of breaking in case of such a client was found, search for suitable client over clients list have to be continued. Otherwise some clients could not be covered by PipeFS event handler. Signed-off-by: Stanislav Kinsbursky Cc: stable@vger.kernel.org [>= v3.4] Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 55e174f2d02f..822f020fa7f4 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -237,7 +237,7 @@ static struct rpc_clnt *rpc_get_client_for_event(struct net *net, int event) spin_lock(&sn->rpc_client_lock); list_for_each_entry(clnt, &sn->all_clients, cl_clients) { if (clnt->cl_program->pipe_dir_name == NULL) - break; + continue; if (rpc_clnt_skip_event(clnt, event)) continue; if (atomic_inc_not_zero(&clnt->cl_count) == 0) -- cgit v1.2.1 From 360e1a534901592b289ba8768fc71b6e6ad49070 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 4 Jan 2013 12:50:30 -0500 Subject: SUNRPC: Partial revert of commit 168e4b39d1afb79a7e3ea6c3bb246b4c82c6bdb9 Partially revert commit (SUNRPC: add WARN_ON_ONCE for potential deadlock). The looping behaviour has been tracked down to a knownn issue with workqueues, and a workaround has now been implemented. Signed-off-by: Trond Myklebust Cc: Weston Andros Adamson Cc: Tejun Heo Cc: Bruce Fields Cc: stable@vger.kernel.org [>= 3.7] --- net/sunrpc/clnt.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 822f020fa7f4..1915ffe598e3 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -610,11 +610,6 @@ EXPORT_SYMBOL_GPL(rpc_killall_tasks); */ void rpc_shutdown_client(struct rpc_clnt *clnt) { - /* - * To avoid deadlock, never call rpc_shutdown_client from a - * workqueue context! - */ - WARN_ON_ONCE(current->flags & PF_WQ_WORKER); might_sleep(); dprintk_rcu("RPC: shutting down %s client for %s\n", -- cgit v1.2.1 From 7144bca6814a79ac86800da9d7f05a8c07bc818c Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Wed, 9 Jan 2013 17:12:35 -0800 Subject: nfs: fix sunrpc/clnt.c kernel-doc warnings Fix new kernel-doc warnings in clnt.c: Warning(net/sunrpc/clnt.c:561): No description found for parameter 'flavor' Warning(net/sunrpc/clnt.c:561): Excess function parameter 'auth' description in 'rpc_clone_client_set_auth' Signed-off-by: Randy Dunlap Cc: Trond Myklebust Cc: "J. Bruce Fields" Cc: linux-nfs@vger.kernel.org Signed-off-by: Linus Torvalds --- net/sunrpc/clnt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 1915ffe598e3..507b5e84fbdb 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -555,7 +555,7 @@ EXPORT_SYMBOL_GPL(rpc_clone_client); * rpc_clone_client_set_auth - Clone an RPC client structure and set its auth * * @clnt: RPC client whose parameters are copied - * @auth: security flavor for new client + * @flavor: security flavor for new client * * Returns a fresh RPC client or an ERR_PTR. */ -- cgit v1.2.1