fix backchannel crash due to multipath
Subsystems (3)
cover fix backchannel crash due to multipath
From: "J. Bruce Fields" <[email protected]>
pynfs can crash the nfs server since the new multipath code. The problem is that pynfs uses the same tcp connection for multiple v4 clients (something the spec explicitly allows). The callback client in such cases has to share the same rpc_xprt. The new rpc code, when it creates two callback clients sharing the same rpc_xprt, ends up trying to keep that rpc_xprt on the lists of two different rpc_xprt_switch's. The first symptom I see is a list corruption warning.
So, I'm enforcing the requirement that there be only one backchannel rpc_xprt_switch per tcp connection by doing as we do in the rpc_xprt
case: keeping a pointer to it in the svc_xprt, and using that when it's available instead of allocating a new one.
That's a pretty straightforward fix (and I've verified it works), but doesn't look very elegant. If there's a better solution, I'm all ears.
--b.
J. Bruce Fields (3):
SUNRPC: fix xprt leak on xps allocation failure nfsd4/rpc: move backchannel create logic into rpc code
rpc: share one xps between all backchannels
fs/nfsd/nfs4callback.c | 18 +----------------- include/linux/sunrpc/clnt.h | 2 -- include/linux/sunrpc/svc_xprt.h | 1 + include/linux/sunrpc/xprt.h | 1 + net/sunrpc/clnt.c | 31 +++++++++++++++++++++++++------ net/sunrpc/svc_xprt.c | 2 ++ net/sunrpc/xprtsock.c | 1 + 7 files changed, 31 insertions(+), 25 deletions(-)
Signature · 1 line
2.5.5
PATCH 1/3 SUNRPC: fix xprt leak on xps allocation failure
From: "J. Bruce Fields" <[email protected]>
Callers of rpc_create_xprt expect it to put the xprt on success and failure.
--- net/sunrpc/clnt.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Alternatively, maybe we should just change that convention and leave it to callers to put the xprt on failure?
net/sunrpc/clnt.c +3 -2
@@ -453,9 +453,10 @@ struct rpc_clnt *rpc_create_xprt(struct rpc_create_args *args,struct rpc_xprt_switch *xps; xps =xprt_switch_alloc (xprt,GFP_KERNEL ); -if (xps ==NULL ) +if (xps ==NULL ) { +xprt_put (xprt);return ERR_PTR (-ENOMEM ); - + } clnt =rpc_new_client (args, xps, xprt,NULL );if (IS_ERR (clnt))return clnt;
Signature · 1 line
2.5.5
PATCH 2/3 nfsd4/rpc: move backchannel create logic into rpc code
In reply to J. Bruce FieldsFrom: "J. Bruce Fields" <[email protected]>
Also simplify the logic a bit.
--- fs/nfsd/nfs4callback.c | 18 +----------------- include/linux/sunrpc/clnt.h | 2 -- net/sunrpc/clnt.c | 12 ++++++++++-- 3 files changed, 11 insertions(+), 21 deletions(-)
fs/nfsd/nfs4callback.c +1 -17
@@ -710,22 +710,6 @@ static struct rpc_cred *get_backchannel_cred(struct nfs4_client *clp, struct rpc } } -static struct rpc_clnt *create_backchannel_client (struct rpc_create_args *args) -{ -struct rpc_xprt *xprt; - -if (args->protocol !=XPRT_TRANSPORT_BC_TCP ) -return rpc_create (args); - - xprt = args->bc_xprt->xpt_bc_xprt; -if (xprt) { -xprt_get (xprt); -return rpc_create_xprt (args, xprt); - } - -return rpc_create (args); -} -static int setup_callback_client (struct nfs4_client *clp,struct nfs4_cb_conn *conn,struct nfsd4_session *ses) {int maxtime =max_cb_time (clp->net);
@@ -768,7 +752,7 @@ static int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *c args.authflavor = ses->se_cb_sec.flavor; }/* Create RPC client */ - client =create_backchannel_client (&args); + client =rpc_create (&args);if (IS_ERR (client)) {dprintk ("NFSD: couldn't create callback client: %ld\n" ,PTR_ERR (client));
include/linux/sunrpc/clnt.h +0 -2
@@ -137,8 +137,6 @@ struct rpc_create_args {#define RPC_CLNT_CREATE_NO_RETRANS_TIMEOUT (1UL << 9)struct rpc_clnt *rpc_create (struct rpc_create_args *args); -struct rpc_clnt *rpc_create_xprt (struct rpc_create_args *args, -struct rpc_xprt *xprt);struct rpc_clnt *rpc_bind_new_program (struct rpc_clnt *,const struct rpc_program *,u32 );struct rpc_clnt *rpc_clone_client (struct rpc_clnt *);
net/sunrpc/clnt.c +10 -2
@@ -446,7 +446,7 @@ out_no_rpciod:return ERR_PTR (err); } -struct rpc_clnt *rpc_create_xprt (struct rpc_create_args *args, +static struct rpc_clnt *rpc_create_xprt (struct rpc_create_args *args,struct rpc_xprt *xprt) {struct rpc_clnt *clnt =NULL ;
@@ -484,7 +484,6 @@ struct rpc_clnt *rpc_create_xprt(struct rpc_create_args *args,return clnt; } -EXPORT_SYMBOL_GPL (rpc_create_xprt);/** * rpc_create - create an RPC client and transport with one call
@@ -510,6 +509,15 @@ struct rpc_clnt *rpc_create(struct rpc_create_args *args) };char servername[48 ]; +if (args->bc_xprt) { +WARN_ON (args->protocol !=XPRT_TRANSPORT_BC_TCP ); + xprt = args->bc_xprt->xpt_bc_xprt; +if (xprt) { +xprt_get (xprt); +return rpc_create_xprt (args, xprt); + } + } +if (args->flags &RPC_CLNT_CREATE_INFINITE_SLOTS ) xprtargs.flags |=XPRT_CREATE_INFINITE_SLOTS ;if (args->flags &RPC_CLNT_CREATE_NO_IDLE_TIMEOUT )
Signature · 1 line
2.5.5
PATCH 3/3 rpc: share one xps between all backchannels
In reply to J. Bruce FieldsFrom: "J. Bruce Fields" <[email protected]>
The spec allows backchannels for multiple clients to share the same tcp connection. When that happens, we need to use the same xprt for all of them. Similarly, we need the same xps.
This fixes list corruption introduced by the multipath code.
--- include/linux/sunrpc/svc_xprt.h | 1 + include/linux/sunrpc/xprt.h | 1 + net/sunrpc/clnt.c | 18 ++++++++++++++---- net/sunrpc/svc_xprt.c | 2 ++ net/sunrpc/xprtsock.c | 1 + 5 files changed, 19 insertions(+), 4 deletions(-)
include/linux/sunrpc/svc_xprt.h +1 -0
@@ -84,6 +84,7 @@ struct svc_xprt {struct net *xpt_net;struct rpc_xprt *xpt_bc_xprt;/* NFSv4.1 backchannel */ +struct rpc_xprt_switch *xpt_bc_xps;/* NFSv4.1 backchannel */ };static inline void unregister_xpt_user (struct svc_xprt *xpt,struct svc_xpt_user *u)
include/linux/sunrpc/xprt.h +1 -0
@@ -296,6 +296,7 @@ struct xprt_create {size_t addrlen;const char *servername;struct svc_xprt *bc_xprt;/* NFSv4.1 backchannel */ +struct rpc_xprt_switch *bc_xps;unsigned int flags; };
net/sunrpc/clnt.c +14 -4
@@ -452,10 +452,20 @@ static struct rpc_clnt *rpc_create_xprt(struct rpc_create_args *args,struct rpc_clnt *clnt =NULL ;struct rpc_xprt_switch *xps; - xps =xprt_switch_alloc (xprt,GFP_KERNEL ); -if (xps ==NULL ) { -xprt_put (xprt); -return ERR_PTR (-ENOMEM ); +if (args->bc_xprt && args->bc_xprt->xpt_bc_xps) { +WARN_ON (args->protocol !=XPRT_TRANSPORT_BC_TCP ); + xps = args->bc_xprt->xpt_bc_xps; +xprt_switch_get (xps); + }else { + xps =xprt_switch_alloc (xprt,GFP_KERNEL ); +if (xps ==NULL ) { +xprt_put (xprt); +return ERR_PTR (-ENOMEM ); + } +if (xprt->bc_xprt) { +xprt_switch_get (xps); + xprt->bc_xprt->xpt_bc_xps = xps; + } } clnt =rpc_new_client (args, xps, xprt,NULL );if (IS_ERR (clnt))
net/sunrpc/svc_xprt.c +2 -0
@@ -136,6 +136,8 @@ static void svc_xprt_free(struct kref *kref)/* See comment on corresponding get in xs_setup_bc_tcp(): */ if (xprt->xpt_bc_xprt)xprt_put (xprt->xpt_bc_xprt); +if (xprt->xpt_bc_xps) +xprt_switch_put (xprt->xpt_bc_xps); xprt->xpt_ops->xpo_free (xprt);module_put (owner); }
net/sunrpc/xprtsock.c +1 -0
@@ -3050,6 +3050,7 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)return xprt; args->bc_xprt->xpt_bc_xprt =NULL ; + args->bc_xprt->xpt_bc_xps =NULL ;xprt_put (xprt); ret =ERR_PTR (-EINVAL ); out_err:
Signature · 1 line
2.5.5