From 384816051ca9125cd54750e59c780c2a2655fa4f Mon Sep 17 00:00:00 2001 From: Stanislav Kinsbursky Date: Mon, 24 Jun 2013 11:52:38 +0400 Subject: SUNRPC: fix races on PipeFS MOUNT notifications Below are races, when RPC client can be created without PiepFS dentries CPU#0 CPU#1 ----------------------------- ----------------------------- rpc_new_client rpc_fill_super rpc_setup_pipedir mutex_lock(&sn->pipefs_sb_lock) rpc_get_sb_net == NULL (no per-net PipeFS superblock) sn->pipefs_sb = sb; notifier_call_chain(MOUNT) (client is not in the list) rpc_register_client (client without pipes dentries) To fix this patch: 1) makes PipeFS mount notification call with pipefs_sb_lock being held. 2) releases pipefs_sb_lock on new SUNRPC client creation only after registration. Signed-off-by: Stanislav Kinsbursky Cc: stable@vger.kernel.org Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 5a750b9c3640..b827a4b91975 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -157,20 +157,15 @@ static struct dentry *rpc_setup_pipedir_sb(struct super_block *sb, } static int -rpc_setup_pipedir(struct rpc_clnt *clnt, const char *dir_name) +rpc_setup_pipedir(struct rpc_clnt *clnt, const char *dir_name, + struct super_block *pipefs_sb) { - struct net *net = rpc_net_ns(clnt); - struct super_block *pipefs_sb; struct dentry *dentry; clnt->cl_dentry = NULL; if (dir_name == NULL) return 0; - pipefs_sb = rpc_get_sb_net(net); - if (!pipefs_sb) - return 0; dentry = rpc_setup_pipedir_sb(pipefs_sb, clnt, dir_name); - rpc_put_sb_net(net); if (IS_ERR(dentry)) return PTR_ERR(dentry); clnt->cl_dentry = dentry; @@ -296,6 +291,7 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args, stru struct rpc_clnt *clnt = NULL; struct rpc_auth *auth; int err; + struct super_block *pipefs_sb; /* sanity check the name before trying to print it */ dprintk("RPC: creating %s client for %s (xprt %p)\n", @@ -354,9 +350,12 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args, stru atomic_set(&clnt->cl_count, 1); - err = rpc_setup_pipedir(clnt, program->pipe_dir_name); - if (err < 0) - goto out_no_path; + pipefs_sb = rpc_get_sb_net(rpc_net_ns(clnt)); + if (pipefs_sb) { + err = rpc_setup_pipedir(clnt, program->pipe_dir_name, pipefs_sb); + if (err) + goto out_no_path; + } auth = rpcauth_create(args->authflavor, clnt); if (IS_ERR(auth)) { @@ -369,11 +368,16 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args, stru /* save the nodename */ rpc_clnt_set_nodename(clnt, utsname()->nodename); rpc_register_client(clnt); + if (pipefs_sb) + rpc_put_sb_net(rpc_net_ns(clnt)); return clnt; out_no_auth: - rpc_clnt_remove_pipedir(clnt); + if (pipefs_sb) + __rpc_clnt_remove_pipedir(clnt); out_no_path: + if (pipefs_sb) + rpc_put_sb_net(rpc_net_ns(clnt)); kfree(clnt->cl_principal); out_no_principal: rpc_free_iostats(clnt->cl_metrics); -- cgit v1.2.3 From adb6fa7ffe9031857ec14b8aab75c9ab65556cbc Mon Sep 17 00:00:00 2001 From: Stanislav Kinsbursky Date: Wed, 26 Jun 2013 10:15:14 +0400 Subject: SUNRPC: fix races on PipeFS UMOUNT notifications CPU#0 CPU#1 ----------------------------- ----------------------------- rpc_kill_sb sn->pipefs_sb = NULL rpc_release_client (UMOUNT_EVENT) rpc_free_auth rpc_pipefs_event rpc_get_client_for_event !atomic_inc_not_zero(cl_count) atomic_inc(cl_count) rpc_free_client rpc_clnt_remove_pipedir To fix this, this patch does the following: 1) Calls RPC_PIPEFS_UMOUNT notification with sn->pipefs_sb_lock being held. 2) Removes SUNRPC client from the list AFTER pipes destroying. 3) Doesn't hold RPC client on notification: if client in the list, then it can't be destroyed while sn->pipefs_sb_lock in hold by notification caller. Signed-off-by: Stanislav Kinsbursky Cc: stable@vger.kernel.org Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index b827a4b91975..41f180c5a498 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -236,8 +236,6 @@ static struct rpc_clnt *rpc_get_client_for_event(struct net *net, int event) continue; if (rpc_clnt_skip_event(clnt, event)) continue; - if (atomic_inc_not_zero(&clnt->cl_count) == 0) - continue; spin_unlock(&sn->rpc_client_lock); return clnt; } @@ -254,7 +252,6 @@ static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event, while ((clnt = rpc_get_client_for_event(sb->s_fs_info, event))) { error = __rpc_pipefs_event(clnt, event, sb); - rpc_release_client(clnt); if (error) break; } @@ -641,8 +638,8 @@ rpc_free_client(struct rpc_clnt *clnt) rcu_dereference(clnt->cl_xprt)->servername); if (clnt->cl_parent != clnt) rpc_release_client(clnt->cl_parent); - rpc_unregister_client(clnt); rpc_clnt_remove_pipedir(clnt); + rpc_unregister_client(clnt); rpc_free_iostats(clnt->cl_metrics); kfree(clnt->cl_principal); clnt->cl_metrics = NULL; -- cgit v1.2.3 From e73f4cc051199799aee4320f300f28ffb82f3eb1 Mon Sep 17 00:00:00 2001 From: Stanislav Kinsbursky Date: Mon, 24 Jun 2013 11:52:52 +0400 Subject: SUNRPC: split client creation routine into setup and registration This helper moves all "registration" code to the new rpc_client_register() helper. This helper will be used later in the series to synchronize against PipeFS MOUNT/UMOUNT events. Signed-off-by: Stanislav Kinsbursky Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 64 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 39 insertions(+), 25 deletions(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 41f180c5a498..b4f17117b779 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -281,14 +281,47 @@ static void rpc_clnt_set_nodename(struct rpc_clnt *clnt, const char *nodename) memcpy(clnt->cl_nodename, nodename, clnt->cl_nodelen); } +static int rpc_client_register(const struct rpc_create_args *args, + struct rpc_clnt *clnt) +{ + const struct rpc_program *program = args->program; + struct rpc_auth *auth; + struct net *net = rpc_net_ns(clnt); + struct super_block *pipefs_sb; + int err = 0; + + pipefs_sb = rpc_get_sb_net(net); + if (pipefs_sb) { + err = rpc_setup_pipedir(clnt, program->pipe_dir_name, pipefs_sb); + if (err) + goto out; + } + + auth = rpcauth_create(args->authflavor, clnt); + if (IS_ERR(auth)) { + dprintk("RPC: Couldn't create auth handle (flavor %u)\n", + args->authflavor); + err = PTR_ERR(auth); + goto err_auth; + } + + rpc_register_client(clnt); +out: + if (pipefs_sb) + rpc_put_sb_net(net); + return err; + +err_auth: + __rpc_clnt_remove_pipedir(clnt); + goto out; +} + static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args, struct rpc_xprt *xprt) { const struct rpc_program *program = args->program; const struct rpc_version *version; struct rpc_clnt *clnt = NULL; - struct rpc_auth *auth; int err; - struct super_block *pipefs_sb; /* sanity check the name before trying to print it */ dprintk("RPC: creating %s client for %s (xprt %p)\n", @@ -347,34 +380,15 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args, stru atomic_set(&clnt->cl_count, 1); - pipefs_sb = rpc_get_sb_net(rpc_net_ns(clnt)); - if (pipefs_sb) { - err = rpc_setup_pipedir(clnt, program->pipe_dir_name, pipefs_sb); - if (err) - goto out_no_path; - } - - auth = rpcauth_create(args->authflavor, clnt); - if (IS_ERR(auth)) { - dprintk("RPC: Couldn't create auth handle (flavor %u)\n", - args->authflavor); - err = PTR_ERR(auth); - goto out_no_auth; - } - /* save the nodename */ rpc_clnt_set_nodename(clnt, utsname()->nodename); - rpc_register_client(clnt); - if (pipefs_sb) - rpc_put_sb_net(rpc_net_ns(clnt)); + + err = rpc_client_register(args, clnt); + if (err) + goto out_no_path; return clnt; -out_no_auth: - if (pipefs_sb) - __rpc_clnt_remove_pipedir(clnt); out_no_path: - if (pipefs_sb) - rpc_put_sb_net(rpc_net_ns(clnt)); kfree(clnt->cl_principal); out_no_principal: rpc_free_iostats(clnt->cl_metrics); -- cgit v1.2.3 From 4f6bb246f69443549fbbd0f2abaf863243cb35e9 Mon Sep 17 00:00:00 2001 From: Stanislav Kinsbursky Date: Mon, 24 Jun 2013 11:52:59 +0400 Subject: SUNRPC: PipeFS MOUNT notification optimization for dying clients Not need to create pipes for dying client. So just skip them. Note: we can safely dereference the client structure, because notification caller is holding sn->pipefs_sb_lock. Signed-off-by: Stanislav Kinsbursky Cc: stable@vger.kernel.org Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index b4f17117b779..f0339ae9bf37 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -177,6 +177,8 @@ static inline int rpc_clnt_skip_event(struct rpc_clnt *clnt, unsigned long event if (((event == RPC_PIPEFS_MOUNT) && clnt->cl_dentry) || ((event == RPC_PIPEFS_UMOUNT) && !clnt->cl_dentry)) return 1; + if ((event == RPC_PIPEFS_MOUNT) && atomic_read(&clnt->cl_count) == 0) + return 1; return 0; } -- cgit v1.2.3 From eeee245268c951262b861bc1be4e9dc812352499 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Wed, 10 Jul 2013 15:33:01 -0400 Subject: SUNRPC: Fix a deadlock in rpc_client_register() Commit 384816051ca9125cd54750e59c780c2a2655fa4f (SUNRPC: fix races on PipeFS MOUNT notifications) introduces a regression when we call rpc_setup_pipedir() with RPCSEC_GSS as the auth flavour. By calling rpcauth_create() while holding the sn->pipefs_sb_lock, we end up deadlocking in gss_pipes_dentries_create_net(). Fix is to register the client and release the mutex before calling rpcauth_create(). Reported-by: Weston Andros Adamson Tested-by: Weston Andros Adamson Cc: Stanislav Kinsbursky Cc: # : 3848160: SUNRPC: fix races on PipeFS MOUNT Cc: # : e73f4cc: SUNRPC: split client creation Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index f0339ae9bf37..aa401560777b 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -290,7 +290,7 @@ static int rpc_client_register(const struct rpc_create_args *args, struct rpc_auth *auth; struct net *net = rpc_net_ns(clnt); struct super_block *pipefs_sb; - int err = 0; + int err; pipefs_sb = rpc_get_sb_net(net); if (pipefs_sb) { @@ -299,6 +299,10 @@ static int rpc_client_register(const struct rpc_create_args *args, goto out; } + rpc_register_client(clnt); + if (pipefs_sb) + rpc_put_sb_net(net); + auth = rpcauth_create(args->authflavor, clnt); if (IS_ERR(auth)) { dprintk("RPC: Couldn't create auth handle (flavor %u)\n", @@ -306,16 +310,14 @@ static int rpc_client_register(const struct rpc_create_args *args, err = PTR_ERR(auth); goto err_auth; } - - rpc_register_client(clnt); + return 0; +err_auth: + pipefs_sb = rpc_get_sb_net(net); + __rpc_clnt_remove_pipedir(clnt); out: if (pipefs_sb) rpc_put_sb_net(net); return err; - -err_auth: - __rpc_clnt_remove_pipedir(clnt); - goto out; } static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args, struct rpc_xprt *xprt) -- cgit v1.2.3 From a95e691f9c4a6e24fdeab6d7feae6d5411fe8a69 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 14 Jul 2013 16:43:54 +0400 Subject: rpc_create_*_dir: don't bother with qstr just pass the name Signed-off-by: Al Viro --- net/sunrpc/clnt.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 5a750b9c3640..26456274b24e 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -128,9 +128,7 @@ static struct dentry *rpc_setup_pipedir_sb(struct super_block *sb, { static uint32_t clntid; char name[15]; - struct qstr q = { .name = name }; struct dentry *dir, *dentry; - int error; dir = rpc_d_lookup_sb(sb, dir_name); if (dir == NULL) { @@ -138,19 +136,17 @@ static struct dentry *rpc_setup_pipedir_sb(struct super_block *sb, return dir; } for (;;) { - q.len = snprintf(name, sizeof(name), "clnt%x", (unsigned int)clntid++); + snprintf(name, sizeof(name), "clnt%x", (unsigned int)clntid++); name[sizeof(name) - 1] = '\0'; - q.hash = full_name_hash(q.name, q.len); - dentry = rpc_create_client_dir(dir, &q, clnt); + dentry = rpc_create_client_dir(dir, name, clnt); if (!IS_ERR(dentry)) break; - error = PTR_ERR(dentry); - if (error != -EEXIST) { - printk(KERN_INFO "RPC: Couldn't create pipefs entry" - " %s/%s, error %d\n", - dir_name, name, error); - break; - } + if (dentry == ERR_PTR(-EEXIST)) + continue; + printk(KERN_INFO "RPC: Couldn't create pipefs entry" + " %s/%s, error %ld\n", + dir_name, name, PTR_ERR(dentry)); + break; } dput(dir); return dentry; -- cgit v1.2.3 From 1540c5d3cbf7670eb68a0d02611ec73e5604a91a Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Sun, 14 Jul 2013 22:57:50 -0400 Subject: SUNRPC: Fix another issue with rpc_client_register() Fix the error pathway if rpcauth_create() fails. 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 9963584605c0..74f6a704e374 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -309,6 +309,7 @@ static int rpc_client_register(const struct rpc_create_args *args, return 0; err_auth: pipefs_sb = rpc_get_sb_net(net); + rpc_unregister_client(clnt); __rpc_clnt_remove_pipedir(clnt); out: if (pipefs_sb) -- cgit v1.2.3 From 786615bc1ce84150ded80daea6bd9f6297f48e73 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Mon, 5 Aug 2013 16:04:47 -0400 Subject: SUNRPC: If the rpcbind channel is disconnected, fail the call to unregister If rpcbind causes our connection to the AF_LOCAL socket to close after we've registered a service, then we want to be careful about reconnecting since the mount namespace may have changed. By simply refusing to reconnect the AF_LOCAL socket in the case of unregister, we avoid the need to somehow save the mount namespace. While this may lead to some services not unregistering properly, it should be safe. Signed-off-by: Trond Myklebust Cc: Nix Cc: Jeff Layton Cc: stable@vger.kernel.org # 3.9.x --- net/sunrpc/clnt.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'net/sunrpc/clnt.c') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 74f6a704e374..ecbc4e3d83ad 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -1660,6 +1660,10 @@ call_connect(struct rpc_task *task) task->tk_action = call_connect_status; if (task->tk_status < 0) return; + if (task->tk_flags & RPC_TASK_NOCONNECT) { + rpc_exit(task, -ENOTCONN); + return; + } xprt_connect(task); } } -- cgit v1.2.3