linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 01/10] sunrpc: use SVC_NET() in svcauth_gss_* functions
       [not found] <cover.1545648882.git.vvs@virtuozzo.com>
@ 2018-12-24 11:44 ` Vasily Averin
  2018-12-24 11:44 ` [PATCH v4 02/10] sunrpc: use-after-free in svc_process_common() Vasily Averin
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Vasily Averin @ 2018-12-24 11:44 UTC (permalink / raw)
  To: J. Bruce Fields, Trond Myklebust
  Cc: Jeff Layton, Anna Schumaker, Chuck Lever, linux-nfs, Evgenii Shatokhin

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 1ece4bc3eb8d..152790ed309c 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1142,7 +1142,7 @@ static int svcauth_gss_legacy_init(struct svc_rqst *rqstp,
 	struct kvec *resv = &rqstp->rq_res.head[0];
 	struct rsi *rsip, rsikey;
 	int ret;
-	struct sunrpc_net *sn = net_generic(rqstp->rq_xprt->xpt_net, sunrpc_net_id);
+	struct sunrpc_net *sn = net_generic(SVC_NET(rqstp), sunrpc_net_id);
 
 	memset(&rsikey, 0, sizeof(rsikey));
 	ret = gss_read_verf(gc, argv, authp,
@@ -1253,7 +1253,7 @@ static int svcauth_gss_proxy_init(struct svc_rqst *rqstp,
 	uint64_t handle;
 	int status;
 	int ret;
-	struct net *net = rqstp->rq_xprt->xpt_net;
+	struct net *net = SVC_NET(rqstp);
 	struct sunrpc_net *sn = net_generic(net, sunrpc_net_id);
 
 	memset(&ud, 0, sizeof(ud));
@@ -1444,7 +1444,7 @@ svcauth_gss_accept(struct svc_rqst *rqstp, __be32 *authp)
 	__be32		*rpcstart;
 	__be32		*reject_stat = resv->iov_base + resv->iov_len;
 	int		ret;
-	struct sunrpc_net *sn = net_generic(rqstp->rq_xprt->xpt_net, sunrpc_net_id);
+	struct sunrpc_net *sn = net_generic(SVC_NET(rqstp), sunrpc_net_id);
 
 	dprintk("RPC:       svcauth_gss: argv->iov_len = %zd\n",
 			argv->iov_len);
@@ -1734,7 +1734,7 @@ svcauth_gss_release(struct svc_rqst *rqstp)
 	struct rpc_gss_wire_cred *gc = &gsd->clcred;
 	struct xdr_buf *resbuf = &rqstp->rq_res;
 	int stat = -EINVAL;
-	struct sunrpc_net *sn = net_generic(rqstp->rq_xprt->xpt_net, sunrpc_net_id);
+	struct sunrpc_net *sn = net_generic(SVC_NET(rqstp), sunrpc_net_id);
 
 	if (gc->gc_proc != RPC_GSS_PROC_DATA)
 		goto out;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 02/10] sunrpc: use-after-free in svc_process_common()
       [not found] <cover.1545648882.git.vvs@virtuozzo.com>
  2018-12-24 11:44 ` [PATCH v4 01/10] sunrpc: use SVC_NET() in svcauth_gss_* functions Vasily Averin
@ 2018-12-24 11:44 ` Vasily Averin
  2018-12-24 16:59   ` J. Bruce Fields
  2018-12-24 11:45 ` [PATCH v4 03/10] sunrpc: replace svc_serv->sv_bc_xprt by boolean flag Vasily Averin
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 11+ messages in thread
From: Vasily Averin @ 2018-12-24 11:44 UTC (permalink / raw)
  To: J. Bruce Fields, Trond Myklebust
  Cc: Jeff Layton, Anna Schumaker, Chuck Lever, linux-nfs, Evgenii Shatokhin

if node have NFSv41+ mounts inside several net namespaces
it can lead to use-after-free in svc_process_common()

svc_process_common()
        /* Setup reply header */
        rqstp->rq_xprt->xpt_ops->xpo_prep_reply_hdr(rqstp); <<< HERE

svc_process_common() can use incorrect rqstp->rq_xprt,
its caller function bc_svc_process() takes it from serv->sv_bc_xprt.
The problem is that serv is global structure but sv_bc_xprt
is assigned per-netnamespace.

According to Trond, the whole "let's set up rqstp->rq_xprt
for the back channel" is nothing but a giant hack in order
to work around the fact that svc_process_common() uses it
to find the xpt_ops, and perform a couple of (meaningless
for the back channel) tests of xpt_flags.

All we really need in svc_process_common() is to be able to run
rqstp->rq_xprt->xpt_ops->xpo_prep_reply_hdr()

Bruce J Fields points that this xpo_prep_reply_hdr() call
is an awfully roundabout way just to do "svc_putnl(resv, 0);"
in the tcp case.

This patch does not initialiuze rqstp->rq_xprt in bc_svc_process(),
now it calls svc_process_common() with rqstp->rq_xprt = NULL.

To adjust reply header svc_process_common() just check
rqstp->rq_prot and calls svc_tcp_prep_reply_hdr() for tcp case.

To handle rqstp->rq_xprt = NULL case in functions called from
svc_process_common() patch intruduces net namespace pointer
svc_rqst->rq_bc_net and adjust SVC_NET() definition.
Some other function was also adopted to properly handle described case.

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 include/linux/sunrpc/svc.h    | 5 ++++-
 include/trace/events/sunrpc.h | 6 ++++--
 net/sunrpc/svc.c              | 9 +++++----
 net/sunrpc/svc_xprt.c         | 5 +++--
 net/sunrpc/svcsock.c          | 2 +-
 5 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 73e130a840ce..fdb6b317d974 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -295,9 +295,12 @@ struct svc_rqst {
 	struct svc_cacherep *	rq_cacherep;	/* cache info */
 	struct task_struct	*rq_task;	/* service thread */
 	spinlock_t		rq_lock;	/* per-request lock */
+	struct net		*rq_bc_net;	/* pointer to backchannel's
+						 * net namespace
+						 */
 };
 
-#define SVC_NET(svc_rqst)	(svc_rqst->rq_xprt->xpt_net)
+#define SVC_NET(rqst) (rqst->rq_xprt ? rqst->rq_xprt->xpt_net : rqst->rq_bc_net)
 
 /*
  * Rigorous type checking on sockaddr type conversions
diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index 28e384186c35..8617f4fd6b70 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -569,7 +569,8 @@ TRACE_EVENT(svc_process,
 		__field(u32, vers)
 		__field(u32, proc)
 		__string(service, name)
-		__string(addr, rqst->rq_xprt->xpt_remotebuf)
+		__string(addr, rqst->rq_xprt ?
+			 rqst->rq_xprt->xpt_remotebuf : "(null)")
 	),
 
 	TP_fast_assign(
@@ -577,7 +578,8 @@ TRACE_EVENT(svc_process,
 		__entry->vers = rqst->rq_vers;
 		__entry->proc = rqst->rq_proc;
 		__assign_str(service, name);
-		__assign_str(addr, rqst->rq_xprt->xpt_remotebuf);
+		__assign_str(addr, rqst->rq_xprt ?
+			     rqst->rq_xprt->xpt_remotebuf : "(null)");
 	),
 
 	TP_printk("addr=%s xid=0x%08x service=%s vers=%u proc=%u",
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index d13e05f1a990..fb647bc01fc5 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1172,7 +1172,8 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	clear_bit(RQ_DROPME, &rqstp->rq_flags);
 
 	/* Setup reply header */
-	rqstp->rq_xprt->xpt_ops->xpo_prep_reply_hdr(rqstp);
+	if (rqstp->rq_prot == IPPROTO_TCP)
+		svc_tcp_prep_reply_hdr(rqstp);
 
 	svc_putu32(resv, rqstp->rq_xid);
 
@@ -1244,7 +1245,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	 * for lower versions. RPC_PROG_MISMATCH seems to be the closest
 	 * fit.
 	 */
-	if (versp->vs_need_cong_ctrl &&
+	if (versp->vs_need_cong_ctrl && rqstp->rq_xprt &&
 	    !test_bit(XPT_CONG_CTRL, &rqstp->rq_xprt->xpt_flags))
 		goto err_bad_vers;
 
@@ -1336,7 +1337,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	return 0;
 
  close:
-	if (test_bit(XPT_TEMP, &rqstp->rq_xprt->xpt_flags))
+	if (rqstp->rq_xprt && test_bit(XPT_TEMP, &rqstp->rq_xprt->xpt_flags))
 		svc_close_xprt(rqstp->rq_xprt);
 	dprintk("svc: svc_process close\n");
 	return 0;
@@ -1459,10 +1460,10 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
 	dprintk("svc: %s(%p)\n", __func__, req);
 
 	/* Build the svc_rqst used by the common processing routine */
-	rqstp->rq_xprt = serv->sv_bc_xprt;
 	rqstp->rq_xid = req->rq_xid;
 	rqstp->rq_prot = req->rq_xprt->prot;
 	rqstp->rq_server = serv;
+	rqstp->rq_bc_net = req->rq_xprt->xprt_net;
 
 	rqstp->rq_addrlen = sizeof(req->rq_xprt->addr);
 	memcpy(&rqstp->rq_addr, &req->rq_xprt->addr, rqstp->rq_addrlen);
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 51d36230b6e3..bd42da287c26 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -468,10 +468,11 @@ static struct svc_xprt *svc_xprt_dequeue(struct svc_pool *pool)
  */
 void svc_reserve(struct svc_rqst *rqstp, int space)
 {
+	struct svc_xprt *xprt = rqstp->rq_xprt;
+
 	space += rqstp->rq_res.head[0].iov_len;
 
-	if (space < rqstp->rq_reserved) {
-		struct svc_xprt *xprt = rqstp->rq_xprt;
+	if (xprt && space < rqstp->rq_reserved) {
 		atomic_sub((rqstp->rq_reserved - space), &xprt->xpt_reserved);
 		rqstp->rq_reserved = space;
 
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 986f3ed7d1a2..793149ba1bda 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1173,7 +1173,7 @@ static int svc_tcp_sendto(struct svc_rqst *rqstp)
 /*
  * Setup response header. TCP has a 4B record length field.
  */
-static void svc_tcp_prep_reply_hdr(struct svc_rqst *rqstp)
+void svc_tcp_prep_reply_hdr(struct svc_rqst *rqstp)
 {
 	struct kvec *resv = &rqstp->rq_res.head[0];
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 03/10] sunrpc: replace svc_serv->sv_bc_xprt by boolean flag
       [not found] <cover.1545648882.git.vvs@virtuozzo.com>
  2018-12-24 11:44 ` [PATCH v4 01/10] sunrpc: use SVC_NET() in svcauth_gss_* functions Vasily Averin
  2018-12-24 11:44 ` [PATCH v4 02/10] sunrpc: use-after-free in svc_process_common() Vasily Averin
@ 2018-12-24 11:45 ` Vasily Averin
  2018-12-24 11:45 ` [PATCH v4 04/10] sunrpc: remove unused bc_up operation from rpc_xprt_ops Vasily Averin
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Vasily Averin @ 2018-12-24 11:45 UTC (permalink / raw)
  To: J. Bruce Fields, Trond Myklebust
  Cc: Jeff Layton, Anna Schumaker, Chuck Lever, linux-nfs, Evgenii Shatokhin

svc_serv-> sv_bc_xprt is netns-unsafe and cannot be used as pointer.
To prevent its misuse in future it is replaced by new boolean flag.

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/nfs/callback.c                        |  8 +++++---
 include/linux/sunrpc/bc_xprt.h           | 10 ++++------
 include/linux/sunrpc/svc.h               |  2 +-
 net/sunrpc/svcsock.c                     |  2 --
 net/sunrpc/xprtrdma/svc_rdma_transport.c |  1 -
 5 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 509dc5adeb8f..6dd04774aedc 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -206,11 +206,13 @@ static int nfs_callback_up_net(int minorversion, struct svc_serv *serv,
 		goto err_bind;
 	}
 
-	ret = -EPROTONOSUPPORT;
+	ret = 0;
 	if (!IS_ENABLED(CONFIG_NFS_V4_1) || minorversion == 0)
 		ret = nfs4_callback_up_net(serv, net);
-	else if (xprt->ops->bc_up)
-		ret = xprt->ops->bc_up(serv, net);
+	else if (xprt->ops->bc_setup)
+		serv->sv_bc_enabled = true;
+	else
+		ret = -EPROTONOSUPPORT;
 
 	if (ret < 0) {
 		printk(KERN_ERR "NFS: callback service start failed\n");
diff --git a/include/linux/sunrpc/bc_xprt.h b/include/linux/sunrpc/bc_xprt.h
index 28721cf73ec3..4e8c773d02be 100644
--- a/include/linux/sunrpc/bc_xprt.h
+++ b/include/linux/sunrpc/bc_xprt.h
@@ -47,11 +47,9 @@ void xprt_free_bc_rqst(struct rpc_rqst *req);
 /*
  * Determine if a shared backchannel is in use
  */
-static inline int svc_is_backchannel(const struct svc_rqst *rqstp)
+static inline bool svc_is_backchannel(const struct svc_rqst *rqstp)
 {
-	if (rqstp->rq_server->sv_bc_xprt)
-		return 1;
-	return 0;
+	return rqstp->rq_server->sv_bc_enabled;
 }
 #else /* CONFIG_SUNRPC_BACKCHANNEL */
 static inline int xprt_setup_backchannel(struct rpc_xprt *xprt,
@@ -60,9 +58,9 @@ static inline int xprt_setup_backchannel(struct rpc_xprt *xprt,
 	return 0;
 }
 
-static inline int svc_is_backchannel(const struct svc_rqst *rqstp)
+static inline bool svc_is_backchannel(const struct svc_rqst *rqstp)
 {
-	return 0;
+	return false;
 }
 
 static inline void xprt_free_bc_request(struct rpc_rqst *req)
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index fdb6b317d974..e52385340b3b 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -109,7 +109,7 @@ struct svc_serv {
 	spinlock_t		sv_cb_lock;	/* protects the svc_cb_list */
 	wait_queue_head_t	sv_cb_waitq;	/* sleep here if there are no
 						 * entries in the svc_cb_list */
-	struct svc_xprt		*sv_bc_xprt;	/* callback on fore channel */
+	bool			sv_bc_enabled;	/* service uses backchannel */
 #endif /* CONFIG_SUNRPC_BACKCHANNEL */
 };
 
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 793149ba1bda..8ce181ecb627 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1623,8 +1623,6 @@ static struct svc_xprt *svc_bc_create_socket(struct svc_serv *serv,
 	svc_xprt_init(net, &svc_tcp_bc_class, xprt, serv);
 	set_bit(XPT_CONG_CTRL, &svsk->sk_xprt.xpt_flags);
 
-	serv->sv_bc_xprt = xprt;
-
 	return xprt;
 }
 
diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index 2f7ec8912f49..d410e6f34f44 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -136,7 +136,6 @@ static struct svc_xprt *svc_rdma_bc_create(struct svc_serv *serv,
 
 	svc_xprt_init(net, &svc_rdma_bc_class, xprt, serv);
 	set_bit(XPT_CONG_CTRL, &xprt->xpt_flags);
-	serv->sv_bc_xprt = xprt;
 
 	dprintk("svcrdma: %s(%p)\n", __func__, xprt);
 	return xprt;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 04/10] sunrpc: remove unused bc_up operation from rpc_xprt_ops
       [not found] <cover.1545648882.git.vvs@virtuozzo.com>
                   ` (2 preceding siblings ...)
  2018-12-24 11:45 ` [PATCH v4 03/10] sunrpc: replace svc_serv->sv_bc_xprt by boolean flag Vasily Averin
@ 2018-12-24 11:45 ` Vasily Averin
  2018-12-24 11:45 ` [PATCH v4 05/10] sunrpc: remove svc_tcp_bc_class Vasily Averin
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Vasily Averin @ 2018-12-24 11:45 UTC (permalink / raw)
  To: J. Bruce Fields, Trond Myklebust
  Cc: Jeff Layton, Anna Schumaker, Chuck Lever, linux-nfs, Evgenii Shatokhin

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 include/linux/sunrpc/xprt.h       |  1 -
 net/sunrpc/xprtrdma/backchannel.c | 20 --------------------
 net/sunrpc/xprtrdma/transport.c   |  1 -
 net/sunrpc/xprtrdma/xprt_rdma.h   |  1 -
 net/sunrpc/xprtsock.c             | 12 ------------
 5 files changed, 35 deletions(-)

diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index a4ab4f8d9140..ad7e910b119d 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -157,7 +157,6 @@ struct rpc_xprt_ops {
 	void		(*inject_disconnect)(struct rpc_xprt *xprt);
 	int		(*bc_setup)(struct rpc_xprt *xprt,
 				    unsigned int min_reqs);
-	int		(*bc_up)(struct svc_serv *serv, struct net *net);
 	size_t		(*bc_maxpayload)(struct rpc_xprt *xprt);
 	void		(*bc_free_rqst)(struct rpc_rqst *rqst);
 	void		(*bc_destroy)(struct rpc_xprt *xprt,
diff --git a/net/sunrpc/xprtrdma/backchannel.c b/net/sunrpc/xprtrdma/backchannel.c
index e5b367a3e517..edba0d35776b 100644
--- a/net/sunrpc/xprtrdma/backchannel.c
+++ b/net/sunrpc/xprtrdma/backchannel.c
@@ -113,26 +113,6 @@ int xprt_rdma_bc_setup(struct rpc_xprt *xprt, unsigned int reqs)
 	return -ENOMEM;
 }
 
-/**
- * xprt_rdma_bc_up - Create transport endpoint for backchannel service
- * @serv: server endpoint
- * @net: network namespace
- *
- * The "xprt" is an implied argument: it supplies the name of the
- * backchannel transport class.
- *
- * Returns zero on success, negative errno on failure
- */
-int xprt_rdma_bc_up(struct svc_serv *serv, struct net *net)
-{
-	int ret;
-
-	ret = svc_create_xprt(serv, "rdma-bc", net, PF_INET, 0, 0);
-	if (ret < 0)
-		return ret;
-	return 0;
-}
-
 /**
  * xprt_rdma_bc_maxpayload - Return maximum backchannel message size
  * @xprt: transport
diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c
index ae2a83828953..9141068693fa 100644
--- a/net/sunrpc/xprtrdma/transport.c
+++ b/net/sunrpc/xprtrdma/transport.c
@@ -827,7 +827,6 @@ static const struct rpc_xprt_ops xprt_rdma_procs = {
 	.inject_disconnect	= xprt_rdma_inject_disconnect,
 #if defined(CONFIG_SUNRPC_BACKCHANNEL)
 	.bc_setup		= xprt_rdma_bc_setup,
-	.bc_up			= xprt_rdma_bc_up,
 	.bc_maxpayload		= xprt_rdma_bc_maxpayload,
 	.bc_free_rqst		= xprt_rdma_bc_free_rqst,
 	.bc_destroy		= xprt_rdma_bc_destroy,
diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
index a13ccb643ce0..9218dbebedce 100644
--- a/net/sunrpc/xprtrdma/xprt_rdma.h
+++ b/net/sunrpc/xprtrdma/xprt_rdma.h
@@ -661,7 +661,6 @@ void xprt_rdma_cleanup(void);
  */
 #if defined(CONFIG_SUNRPC_BACKCHANNEL)
 int xprt_rdma_bc_setup(struct rpc_xprt *, unsigned int);
-int xprt_rdma_bc_up(struct svc_serv *, struct net *);
 size_t xprt_rdma_bc_maxpayload(struct rpc_xprt *);
 int rpcrdma_bc_post_recv(struct rpcrdma_xprt *, unsigned int);
 void rpcrdma_bc_receive_call(struct rpcrdma_xprt *, struct rpcrdma_rep *);
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 8a5e823e0b33..a51152af8780 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -1400,17 +1400,6 @@ static void xs_tcp_force_close(struct rpc_xprt *xprt)
 }
 
 #if defined(CONFIG_SUNRPC_BACKCHANNEL)
-static int xs_tcp_bc_up(struct svc_serv *serv, struct net *net)
-{
-	int ret;
-
-	ret = svc_create_xprt(serv, "tcp-bc", net, PF_INET, 0,
-			      SVC_SOCK_ANONYMOUS);
-	if (ret < 0)
-		return ret;
-	return 0;
-}
-
 static size_t xs_tcp_bc_maxpayload(struct rpc_xprt *xprt)
 {
 	return PAGE_SIZE;
@@ -2667,7 +2656,6 @@ static const struct rpc_xprt_ops xs_tcp_ops = {
 	.inject_disconnect	= xs_inject_disconnect,
 #ifdef CONFIG_SUNRPC_BACKCHANNEL
 	.bc_setup		= xprt_setup_bc,
-	.bc_up			= xs_tcp_bc_up,
 	.bc_maxpayload		= xs_tcp_bc_maxpayload,
 	.bc_free_rqst		= xprt_free_bc_rqst,
 	.bc_destroy		= xprt_destroy_bc,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 05/10] sunrpc: remove svc_tcp_bc_class
       [not found] <cover.1545648882.git.vvs@virtuozzo.com>
                   ` (3 preceding siblings ...)
  2018-12-24 11:45 ` [PATCH v4 04/10] sunrpc: remove unused bc_up operation from rpc_xprt_ops Vasily Averin
@ 2018-12-24 11:45 ` Vasily Averin
  2018-12-24 11:45 ` [PATCH v4 06/10] sunrpc: remove svc_rdma_bc_class Vasily Averin
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Vasily Averin @ 2018-12-24 11:45 UTC (permalink / raw)
  To: J. Bruce Fields, Trond Myklebust
  Cc: Jeff Layton, Anna Schumaker, Chuck Lever, linux-nfs, Evgenii Shatokhin

Remove svc_xprt_class svc_tcp_bc_class and related functions

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 net/sunrpc/svcsock.c | 101 -------------------------------------------
 1 file changed, 101 deletions(-)

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 8ce181ecb627..19acf10dfca1 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -70,13 +70,6 @@ static void		svc_sock_free(struct svc_xprt *);
 static struct svc_xprt *svc_create_socket(struct svc_serv *, int,
 					  struct net *, struct sockaddr *,
 					  int, int);
-#if defined(CONFIG_SUNRPC_BACKCHANNEL)
-static struct svc_xprt *svc_bc_create_socket(struct svc_serv *, int,
-					     struct net *, struct sockaddr *,
-					     int, int);
-static void svc_bc_sock_free(struct svc_xprt *xprt);
-#endif /* CONFIG_SUNRPC_BACKCHANNEL */
-
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 static struct lock_class_key svc_key[2];
 static struct lock_class_key svc_slock_key[2];
@@ -1189,58 +1182,6 @@ static struct svc_xprt *svc_tcp_create(struct svc_serv *serv,
 	return svc_create_socket(serv, IPPROTO_TCP, net, sa, salen, flags);
 }
 
-#if defined(CONFIG_SUNRPC_BACKCHANNEL)
-static struct svc_xprt *svc_bc_create_socket(struct svc_serv *, int,
-					     struct net *, struct sockaddr *,
-					     int, int);
-static void svc_bc_sock_free(struct svc_xprt *xprt);
-
-static struct svc_xprt *svc_bc_tcp_create(struct svc_serv *serv,
-				       struct net *net,
-				       struct sockaddr *sa, int salen,
-				       int flags)
-{
-	return svc_bc_create_socket(serv, IPPROTO_TCP, net, sa, salen, flags);
-}
-
-static void svc_bc_tcp_sock_detach(struct svc_xprt *xprt)
-{
-}
-
-static const struct svc_xprt_ops svc_tcp_bc_ops = {
-	.xpo_create = svc_bc_tcp_create,
-	.xpo_detach = svc_bc_tcp_sock_detach,
-	.xpo_free = svc_bc_sock_free,
-	.xpo_prep_reply_hdr = svc_tcp_prep_reply_hdr,
-	.xpo_secure_port = svc_sock_secure_port,
-};
-
-static struct svc_xprt_class svc_tcp_bc_class = {
-	.xcl_name = "tcp-bc",
-	.xcl_owner = THIS_MODULE,
-	.xcl_ops = &svc_tcp_bc_ops,
-	.xcl_max_payload = RPCSVC_MAXPAYLOAD_TCP,
-};
-
-static void svc_init_bc_xprt_sock(void)
-{
-	svc_reg_xprt_class(&svc_tcp_bc_class);
-}
-
-static void svc_cleanup_bc_xprt_sock(void)
-{
-	svc_unreg_xprt_class(&svc_tcp_bc_class);
-}
-#else /* CONFIG_SUNRPC_BACKCHANNEL */
-static void svc_init_bc_xprt_sock(void)
-{
-}
-
-static void svc_cleanup_bc_xprt_sock(void)
-{
-}
-#endif /* CONFIG_SUNRPC_BACKCHANNEL */
-
 static const struct svc_xprt_ops svc_tcp_ops = {
 	.xpo_create = svc_tcp_create,
 	.xpo_recvfrom = svc_tcp_recvfrom,
@@ -1267,14 +1208,12 @@ void svc_init_xprt_sock(void)
 {
 	svc_reg_xprt_class(&svc_tcp_class);
 	svc_reg_xprt_class(&svc_udp_class);
-	svc_init_bc_xprt_sock();
 }
 
 void svc_cleanup_xprt_sock(void)
 {
 	svc_unreg_xprt_class(&svc_tcp_class);
 	svc_unreg_xprt_class(&svc_udp_class);
-	svc_cleanup_bc_xprt_sock();
 }
 
 static void svc_tcp_init(struct svc_sock *svsk, struct svc_serv *serv)
@@ -1595,43 +1534,3 @@ static void svc_sock_free(struct svc_xprt *xprt)
 		sock_release(svsk->sk_sock);
 	kfree(svsk);
 }
-
-#if defined(CONFIG_SUNRPC_BACKCHANNEL)
-/*
- * Create a back channel svc_xprt which shares the fore channel socket.
- */
-static struct svc_xprt *svc_bc_create_socket(struct svc_serv *serv,
-					     int protocol,
-					     struct net *net,
-					     struct sockaddr *sin, int len,
-					     int flags)
-{
-	struct svc_sock *svsk;
-	struct svc_xprt *xprt;
-
-	if (protocol != IPPROTO_TCP) {
-		printk(KERN_WARNING "svc: only TCP sockets"
-			" supported on shared back channel\n");
-		return ERR_PTR(-EINVAL);
-	}
-
-	svsk = kzalloc(sizeof(*svsk), GFP_KERNEL);
-	if (!svsk)
-		return ERR_PTR(-ENOMEM);
-
-	xprt = &svsk->sk_xprt;
-	svc_xprt_init(net, &svc_tcp_bc_class, xprt, serv);
-	set_bit(XPT_CONG_CTRL, &svsk->sk_xprt.xpt_flags);
-
-	return xprt;
-}
-
-/*
- * Free a back channel svc_sock.
- */
-static void svc_bc_sock_free(struct svc_xprt *xprt)
-{
-	if (xprt)
-		kfree(container_of(xprt, struct svc_sock, sk_xprt));
-}
-#endif /* CONFIG_SUNRPC_BACKCHANNEL */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 06/10] sunrpc: remove svc_rdma_bc_class
       [not found] <cover.1545648882.git.vvs@virtuozzo.com>
                   ` (4 preceding siblings ...)
  2018-12-24 11:45 ` [PATCH v4 05/10] sunrpc: remove svc_tcp_bc_class Vasily Averin
@ 2018-12-24 11:45 ` Vasily Averin
  2018-12-24 11:46 ` [PATCH v4 07/10] sunrpc: remove unused xpo_prep_reply_hdr callback Vasily Averin
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Vasily Averin @ 2018-12-24 11:45 UTC (permalink / raw)
  To: J. Bruce Fields, Trond Myklebust
  Cc: Jeff Layton, Anna Schumaker, Chuck Lever, linux-nfs, Evgenii Shatokhin

Remove svc_xprt_class svc_rdma_bc_class and related functions.

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 net/sunrpc/xprtrdma/svc_rdma.c           |  6 ---
 net/sunrpc/xprtrdma/svc_rdma_transport.c | 57 ------------------------
 2 files changed, 63 deletions(-)

diff --git a/net/sunrpc/xprtrdma/svc_rdma.c b/net/sunrpc/xprtrdma/svc_rdma.c
index 134bef6a451e..abdb3004a1e3 100644
--- a/net/sunrpc/xprtrdma/svc_rdma.c
+++ b/net/sunrpc/xprtrdma/svc_rdma.c
@@ -235,9 +235,6 @@ void svc_rdma_cleanup(void)
 		unregister_sysctl_table(svcrdma_table_header);
 		svcrdma_table_header = NULL;
 	}
-#if defined(CONFIG_SUNRPC_BACKCHANNEL)
-	svc_unreg_xprt_class(&svc_rdma_bc_class);
-#endif
 	svc_unreg_xprt_class(&svc_rdma_class);
 }
 
@@ -259,8 +256,5 @@ int svc_rdma_init(void)
 
 	/* Register RDMA with the SVC transport switch */
 	svc_reg_xprt_class(&svc_rdma_class);
-#if defined(CONFIG_SUNRPC_BACKCHANNEL)
-	svc_reg_xprt_class(&svc_rdma_bc_class);
-#endif
 	return 0;
 }
diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index d410e6f34f44..085933cc6b3e 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -100,63 +100,6 @@ struct svc_xprt_class svc_rdma_class = {
 	.xcl_ident = XPRT_TRANSPORT_RDMA,
 };
 
-#if defined(CONFIG_SUNRPC_BACKCHANNEL)
-static struct svc_xprt *svc_rdma_bc_create(struct svc_serv *, struct net *,
-					   struct sockaddr *, int, int);
-static void svc_rdma_bc_detach(struct svc_xprt *);
-static void svc_rdma_bc_free(struct svc_xprt *);
-
-static const struct svc_xprt_ops svc_rdma_bc_ops = {
-	.xpo_create = svc_rdma_bc_create,
-	.xpo_detach = svc_rdma_bc_detach,
-	.xpo_free = svc_rdma_bc_free,
-	.xpo_prep_reply_hdr = svc_rdma_prep_reply_hdr,
-	.xpo_secure_port = svc_rdma_secure_port,
-};
-
-struct svc_xprt_class svc_rdma_bc_class = {
-	.xcl_name = "rdma-bc",
-	.xcl_owner = THIS_MODULE,
-	.xcl_ops = &svc_rdma_bc_ops,
-	.xcl_max_payload = (1024 - RPCRDMA_HDRLEN_MIN)
-};
-
-static struct svc_xprt *svc_rdma_bc_create(struct svc_serv *serv,
-					   struct net *net,
-					   struct sockaddr *sa, int salen,
-					   int flags)
-{
-	struct svcxprt_rdma *cma_xprt;
-	struct svc_xprt *xprt;
-
-	cma_xprt = svc_rdma_create_xprt(serv, net);
-	if (!cma_xprt)
-		return ERR_PTR(-ENOMEM);
-	xprt = &cma_xprt->sc_xprt;
-
-	svc_xprt_init(net, &svc_rdma_bc_class, xprt, serv);
-	set_bit(XPT_CONG_CTRL, &xprt->xpt_flags);
-
-	dprintk("svcrdma: %s(%p)\n", __func__, xprt);
-	return xprt;
-}
-
-static void svc_rdma_bc_detach(struct svc_xprt *xprt)
-{
-	dprintk("svcrdma: %s(%p)\n", __func__, xprt);
-}
-
-static void svc_rdma_bc_free(struct svc_xprt *xprt)
-{
-	struct svcxprt_rdma *rdma =
-		container_of(xprt, struct svcxprt_rdma, sc_xprt);
-
-	dprintk("svcrdma: %s(%p)\n", __func__, xprt);
-	if (xprt)
-		kfree(rdma);
-}
-#endif	/* CONFIG_SUNRPC_BACKCHANNEL */
-
 /* QP event handler */
 static void qp_event_handler(struct ib_event *event, void *context)
 {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 07/10] sunrpc: remove unused xpo_prep_reply_hdr callback
       [not found] <cover.1545648882.git.vvs@virtuozzo.com>
                   ` (5 preceding siblings ...)
  2018-12-24 11:45 ` [PATCH v4 06/10] sunrpc: remove svc_rdma_bc_class Vasily Averin
@ 2018-12-24 11:46 ` Vasily Averin
  2018-12-24 11:46 ` [PATCH v4 08/10] sunrpc: make visible processing error in bc_svc_process() Vasily Averin
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Vasily Averin @ 2018-12-24 11:46 UTC (permalink / raw)
  To: J. Bruce Fields, Trond Myklebust
  Cc: Jeff Layton, Anna Schumaker, Chuck Lever, linux-nfs, Evgenii Shatokhin

xpo_prep_reply_hdr are not used now.

It was defined for tcp transport only, however it cannot be
called indirectly, so let's move it to its caller and
remove unused callback.

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 include/linux/sunrpc/svc_rdma.h          |  1 -
 include/linux/sunrpc/svc_xprt.h          |  1 -
 net/sunrpc/svc.c                         | 11 +++++++++++
 net/sunrpc/svcsock.c                     | 17 -----------------
 net/sunrpc/xprtrdma/svc_rdma_sendto.c    |  4 ----
 net/sunrpc/xprtrdma/svc_rdma_transport.c |  1 -
 6 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
index e6e26918504c..cc2f25a10dcf 100644
--- a/include/linux/sunrpc/svc_rdma.h
+++ b/include/linux/sunrpc/svc_rdma.h
@@ -192,7 +192,6 @@ extern int svc_rdma_sendto(struct svc_rqst *);
 extern int svc_rdma_create_listen(struct svc_serv *, int, struct sockaddr *);
 extern void svc_sq_reap(struct svcxprt_rdma *);
 extern void svc_rq_reap(struct svcxprt_rdma *);
-extern void svc_rdma_prep_reply_hdr(struct svc_rqst *);
 
 extern struct svc_xprt_class svc_rdma_class;
 #ifdef CONFIG_SUNRPC_BACKCHANNEL
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 6b7a86c4d6e6..b3f9577e17d6 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -20,7 +20,6 @@ struct svc_xprt_ops {
 	struct svc_xprt	*(*xpo_accept)(struct svc_xprt *);
 	int		(*xpo_has_wspace)(struct svc_xprt *);
 	int		(*xpo_recvfrom)(struct svc_rqst *);
-	void		(*xpo_prep_reply_hdr)(struct svc_rqst *);
 	int		(*xpo_sendto)(struct svc_rqst *);
 	void		(*xpo_release_rqst)(struct svc_rqst *);
 	void		(*xpo_detach)(struct svc_xprt *);
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index fb647bc01fc5..1e6701c065f9 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1144,6 +1144,17 @@ void svc_printk(struct svc_rqst *rqstp, const char *fmt, ...)
 static __printf(2,3) void svc_printk(struct svc_rqst *rqstp, const char *fmt, ...) {}
 #endif
 
+/*
+ * Setup response header for TCP, it has a 4B record length field.
+ */
+static void svc_tcp_prep_reply_hdr(struct svc_rqst *rqstp)
+{
+	struct kvec *resv = &rqstp->rq_res.head[0];
+
+	/* tcp needs a space for the record length... */
+	svc_putnl(resv, 0);
+}
+
 /*
  * Common routine for processing the RPC request.
  */
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 19acf10dfca1..c7ae1ed5324f 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -610,10 +610,6 @@ svc_udp_sendto(struct svc_rqst *rqstp)
 	return error;
 }
 
-static void svc_udp_prep_reply_hdr(struct svc_rqst *rqstp)
-{
-}
-
 static int svc_udp_has_wspace(struct svc_xprt *xprt)
 {
 	struct svc_sock *svsk = container_of(xprt, struct svc_sock, sk_xprt);
@@ -657,7 +653,6 @@ static const struct svc_xprt_ops svc_udp_ops = {
 	.xpo_release_rqst = svc_release_udp_skb,
 	.xpo_detach = svc_sock_detach,
 	.xpo_free = svc_sock_free,
-	.xpo_prep_reply_hdr = svc_udp_prep_reply_hdr,
 	.xpo_has_wspace = svc_udp_has_wspace,
 	.xpo_accept = svc_udp_accept,
 	.xpo_secure_port = svc_sock_secure_port,
@@ -1163,17 +1158,6 @@ static int svc_tcp_sendto(struct svc_rqst *rqstp)
 	return sent;
 }
 
-/*
- * Setup response header. TCP has a 4B record length field.
- */
-void svc_tcp_prep_reply_hdr(struct svc_rqst *rqstp)
-{
-	struct kvec *resv = &rqstp->rq_res.head[0];
-
-	/* tcp needs a space for the record length... */
-	svc_putnl(resv, 0);
-}
-
 static struct svc_xprt *svc_tcp_create(struct svc_serv *serv,
 				       struct net *net,
 				       struct sockaddr *sa, int salen,
@@ -1189,7 +1173,6 @@ static const struct svc_xprt_ops svc_tcp_ops = {
 	.xpo_release_rqst = svc_release_skb,
 	.xpo_detach = svc_tcp_sock_detach,
 	.xpo_free = svc_sock_free,
-	.xpo_prep_reply_hdr = svc_tcp_prep_reply_hdr,
 	.xpo_has_wspace = svc_tcp_has_wspace,
 	.xpo_accept = svc_tcp_accept,
 	.xpo_secure_port = svc_sock_secure_port,
diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
index 8602a5f1b515..e8016c428890 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
@@ -741,10 +741,6 @@ static int svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
 	return 0;
 }
 
-void svc_rdma_prep_reply_hdr(struct svc_rqst *rqstp)
-{
-}
-
 /**
  * svc_rdma_sendto - Transmit an RPC reply
  * @rqstp: processed RPC request, reply XDR already in ::rq_res
diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index 085933cc6b3e..924c17d46903 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -85,7 +85,6 @@ static const struct svc_xprt_ops svc_rdma_ops = {
 	.xpo_release_rqst = svc_rdma_release_rqst,
 	.xpo_detach = svc_rdma_detach,
 	.xpo_free = svc_rdma_free,
-	.xpo_prep_reply_hdr = svc_rdma_prep_reply_hdr,
 	.xpo_has_wspace = svc_rdma_has_wspace,
 	.xpo_accept = svc_rdma_accept,
 	.xpo_secure_port = svc_rdma_secure_port,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 08/10] sunrpc: make visible processing error in bc_svc_process()
       [not found] <cover.1545648882.git.vvs@virtuozzo.com>
                   ` (6 preceding siblings ...)
  2018-12-24 11:46 ` [PATCH v4 07/10] sunrpc: remove unused xpo_prep_reply_hdr callback Vasily Averin
@ 2018-12-24 11:46 ` Vasily Averin
  2018-12-24 11:46 ` [PATCH v4 09/10] sunrpc: fix debug message in svc_create_xprt() Vasily Averin
  2018-12-24 11:46 ` [PATCH v4 10/10] nfs: minor typo in nfs4_callback_up_net() Vasily Averin
  9 siblings, 0 replies; 11+ messages in thread
From: Vasily Averin @ 2018-12-24 11:46 UTC (permalink / raw)
  To: J. Bruce Fields, Trond Myklebust
  Cc: Jeff Layton, Anna Schumaker, Chuck Lever, linux-nfs, Evgenii Shatokhin

Force bc_svc_process() to generate debug message after processing errors

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 net/sunrpc/svc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 1e6701c065f9..e87ddb9f7feb 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1511,9 +1511,9 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
 	if (!proc_error) {
 		/* Processing error: drop the request */
 		xprt_free_bc_request(req);
-		return 0;
+		error = -EINVAL;
+		goto out;
 	}
-
 	/* Finally, send the reply synchronously */
 	memcpy(&req->rq_snd_buf, &rqstp->rq_res, sizeof(req->rq_snd_buf));
 	task = rpc_run_bc_task(req);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 09/10] sunrpc: fix debug message in svc_create_xprt()
       [not found] <cover.1545648882.git.vvs@virtuozzo.com>
                   ` (7 preceding siblings ...)
  2018-12-24 11:46 ` [PATCH v4 08/10] sunrpc: make visible processing error in bc_svc_process() Vasily Averin
@ 2018-12-24 11:46 ` Vasily Averin
  2018-12-24 11:46 ` [PATCH v4 10/10] nfs: minor typo in nfs4_callback_up_net() Vasily Averin
  9 siblings, 0 replies; 11+ messages in thread
From: Vasily Averin @ 2018-12-24 11:46 UTC (permalink / raw)
  To: J. Bruce Fields, Trond Myklebust
  Cc: Jeff Layton, Anna Schumaker, Chuck Lever, linux-nfs, Evgenii Shatokhin

_svc_create_xprt() returns positive port number
so its non-zero return value is not an error

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 net/sunrpc/svc_xprt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index bd42da287c26..4eb8fbf2508d 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -296,9 +296,9 @@ int svc_create_xprt(struct svc_serv *serv, const char *xprt_name,
 		request_module("svc%s", xprt_name);
 		err = _svc_create_xprt(serv, xprt_name, net, family, port, flags);
 	}
-	if (err)
+	if (err < 0)
 		dprintk("svc: transport %s not found, err %d\n",
-			xprt_name, err);
+			xprt_name, -err);
 	return err;
 }
 EXPORT_SYMBOL_GPL(svc_create_xprt);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 10/10] nfs: minor typo in nfs4_callback_up_net()
       [not found] <cover.1545648882.git.vvs@virtuozzo.com>
                   ` (8 preceding siblings ...)
  2018-12-24 11:46 ` [PATCH v4 09/10] sunrpc: fix debug message in svc_create_xprt() Vasily Averin
@ 2018-12-24 11:46 ` Vasily Averin
  9 siblings, 0 replies; 11+ messages in thread
From: Vasily Averin @ 2018-12-24 11:46 UTC (permalink / raw)
  To: J. Bruce Fields, Trond Myklebust
  Cc: Jeff Layton, Anna Schumaker, Chuck Lever, linux-nfs, Evgenii Shatokhin

Closing ")" was lost in debug message.

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/nfs/callback.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 6dd04774aedc..82fa65da741b 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -56,7 +56,7 @@ static int nfs4_callback_up_net(struct svc_serv *serv, struct net *net)
 				nfs_callback_set_tcpport, SVC_SOCK_ANONYMOUS);
 	if (ret > 0) {
 		nn->nfs_callback_tcpport6 = ret;
-		dprintk("NFS: Callback listener port = %u (af %u, net %x\n",
+		dprintk("NFS: Callback listener port = %u (af %u, net %x)\n",
 			nn->nfs_callback_tcpport6, PF_INET6, net->ns.inum);
 	} else if (ret != -EAFNOSUPPORT)
 		goto out_err;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 02/10] sunrpc: use-after-free in svc_process_common()
  2018-12-24 11:44 ` [PATCH v4 02/10] sunrpc: use-after-free in svc_process_common() Vasily Averin
@ 2018-12-24 16:59   ` J. Bruce Fields
  0 siblings, 0 replies; 11+ messages in thread
From: J. Bruce Fields @ 2018-12-24 16:59 UTC (permalink / raw)
  To: Vasily Averin
  Cc: Trond Myklebust, Jeff Layton, Anna Schumaker, Chuck Lever,
	linux-nfs, Evgenii Shatokhin

On Mon, Dec 24, 2018 at 02:44:52PM +0300, Vasily Averin wrote:
> if node have NFSv41+ mounts inside several net namespaces
> it can lead to use-after-free in svc_process_common()
> 
> svc_process_common()
>         /* Setup reply header */
>         rqstp->rq_xprt->xpt_ops->xpo_prep_reply_hdr(rqstp); <<< HERE
> 
> svc_process_common() can use incorrect rqstp->rq_xprt,
> its caller function bc_svc_process() takes it from serv->sv_bc_xprt.
> The problem is that serv is global structure but sv_bc_xprt
> is assigned per-netnamespace.
> 
> According to Trond, the whole "let's set up rqstp->rq_xprt
> for the back channel" is nothing but a giant hack in order
> to work around the fact that svc_process_common() uses it
> to find the xpt_ops, and perform a couple of (meaningless
> for the back channel) tests of xpt_flags.
> 
> All we really need in svc_process_common() is to be able to run
> rqstp->rq_xprt->xpt_ops->xpo_prep_reply_hdr()
> 
> Bruce J Fields points that this xpo_prep_reply_hdr() call
> is an awfully roundabout way just to do "svc_putnl(resv, 0);"
> in the tcp case.
> 
> This patch does not initialiuze rqstp->rq_xprt in bc_svc_process(),
> now it calls svc_process_common() with rqstp->rq_xprt = NULL.
> 
> To adjust reply header svc_process_common() just check
> rqstp->rq_prot and calls svc_tcp_prep_reply_hdr() for tcp case.
> 
> To handle rqstp->rq_xprt = NULL case in functions called from
> svc_process_common() patch intruduces net namespace pointer
> svc_rqst->rq_bc_net and adjust SVC_NET() definition.
> Some other function was also adopted to properly handle described case.
> 
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
> ---
>  include/linux/sunrpc/svc.h    | 5 ++++-
>  include/trace/events/sunrpc.h | 6 ++++--
>  net/sunrpc/svc.c              | 9 +++++----
>  net/sunrpc/svc_xprt.c         | 5 +++--
>  net/sunrpc/svcsock.c          | 2 +-
>  5 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index 73e130a840ce..fdb6b317d974 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -295,9 +295,12 @@ struct svc_rqst {
>  	struct svc_cacherep *	rq_cacherep;	/* cache info */
>  	struct task_struct	*rq_task;	/* service thread */
>  	spinlock_t		rq_lock;	/* per-request lock */
> +	struct net		*rq_bc_net;	/* pointer to backchannel's
> +						 * net namespace
> +						 */
>  };
>  
> -#define SVC_NET(svc_rqst)	(svc_rqst->rq_xprt->xpt_net)
> +#define SVC_NET(rqst) (rqst->rq_xprt ? rqst->rq_xprt->xpt_net : rqst->rq_bc_net)
>  
>  /*
>   * Rigorous type checking on sockaddr type conversions
> diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
> index 28e384186c35..8617f4fd6b70 100644
> --- a/include/trace/events/sunrpc.h
> +++ b/include/trace/events/sunrpc.h
> @@ -569,7 +569,8 @@ TRACE_EVENT(svc_process,
>  		__field(u32, vers)
>  		__field(u32, proc)
>  		__string(service, name)
> -		__string(addr, rqst->rq_xprt->xpt_remotebuf)
> +		__string(addr, rqst->rq_xprt ?
> +			 rqst->rq_xprt->xpt_remotebuf : "(null)")
>  	),
>  
>  	TP_fast_assign(
> @@ -577,7 +578,8 @@ TRACE_EVENT(svc_process,
>  		__entry->vers = rqst->rq_vers;
>  		__entry->proc = rqst->rq_proc;
>  		__assign_str(service, name);
> -		__assign_str(addr, rqst->rq_xprt->xpt_remotebuf);
> +		__assign_str(addr, rqst->rq_xprt ?
> +			     rqst->rq_xprt->xpt_remotebuf : "(null)");
>  	),
>  
>  	TP_printk("addr=%s xid=0x%08x service=%s vers=%u proc=%u",
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index d13e05f1a990..fb647bc01fc5 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -1172,7 +1172,8 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
>  	clear_bit(RQ_DROPME, &rqstp->rq_flags);
>  
>  	/* Setup reply header */
> -	rqstp->rq_xprt->xpt_ops->xpo_prep_reply_hdr(rqstp);
> +	if (rqstp->rq_prot == IPPROTO_TCP)
> +		svc_tcp_prep_reply_hdr(rqstp);

svc_tcp_prep_reply_hdr is a one-liner, I'd just copy the svc_putnl() and
comment here.

--b.

>  
>  	svc_putu32(resv, rqstp->rq_xid);
>  
> @@ -1244,7 +1245,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
>  	 * for lower versions. RPC_PROG_MISMATCH seems to be the closest
>  	 * fit.
>  	 */
> -	if (versp->vs_need_cong_ctrl &&
> +	if (versp->vs_need_cong_ctrl && rqstp->rq_xprt &&
>  	    !test_bit(XPT_CONG_CTRL, &rqstp->rq_xprt->xpt_flags))
>  		goto err_bad_vers;
>  
> @@ -1336,7 +1337,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
>  	return 0;
>  
>   close:
> -	if (test_bit(XPT_TEMP, &rqstp->rq_xprt->xpt_flags))
> +	if (rqstp->rq_xprt && test_bit(XPT_TEMP, &rqstp->rq_xprt->xpt_flags))
>  		svc_close_xprt(rqstp->rq_xprt);
>  	dprintk("svc: svc_process close\n");
>  	return 0;
> @@ -1459,10 +1460,10 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
>  	dprintk("svc: %s(%p)\n", __func__, req);
>  
>  	/* Build the svc_rqst used by the common processing routine */
> -	rqstp->rq_xprt = serv->sv_bc_xprt;
>  	rqstp->rq_xid = req->rq_xid;
>  	rqstp->rq_prot = req->rq_xprt->prot;
>  	rqstp->rq_server = serv;
> +	rqstp->rq_bc_net = req->rq_xprt->xprt_net;
>  
>  	rqstp->rq_addrlen = sizeof(req->rq_xprt->addr);
>  	memcpy(&rqstp->rq_addr, &req->rq_xprt->addr, rqstp->rq_addrlen);
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index 51d36230b6e3..bd42da287c26 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -468,10 +468,11 @@ static struct svc_xprt *svc_xprt_dequeue(struct svc_pool *pool)
>   */
>  void svc_reserve(struct svc_rqst *rqstp, int space)
>  {
> +	struct svc_xprt *xprt = rqstp->rq_xprt;
> +
>  	space += rqstp->rq_res.head[0].iov_len;
>  
> -	if (space < rqstp->rq_reserved) {
> -		struct svc_xprt *xprt = rqstp->rq_xprt;
> +	if (xprt && space < rqstp->rq_reserved) {
>  		atomic_sub((rqstp->rq_reserved - space), &xprt->xpt_reserved);
>  		rqstp->rq_reserved = space;
>  
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index 986f3ed7d1a2..793149ba1bda 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -1173,7 +1173,7 @@ static int svc_tcp_sendto(struct svc_rqst *rqstp)
>  /*
>   * Setup response header. TCP has a 4B record length field.
>   */
> -static void svc_tcp_prep_reply_hdr(struct svc_rqst *rqstp)
> +void svc_tcp_prep_reply_hdr(struct svc_rqst *rqstp)
>  {
>  	struct kvec *resv = &rqstp->rq_res.head[0];
>  
> -- 
> 2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-12-24 16:59 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <cover.1545648882.git.vvs@virtuozzo.com>
2018-12-24 11:44 ` [PATCH v4 01/10] sunrpc: use SVC_NET() in svcauth_gss_* functions Vasily Averin
2018-12-24 11:44 ` [PATCH v4 02/10] sunrpc: use-after-free in svc_process_common() Vasily Averin
2018-12-24 16:59   ` J. Bruce Fields
2018-12-24 11:45 ` [PATCH v4 03/10] sunrpc: replace svc_serv->sv_bc_xprt by boolean flag Vasily Averin
2018-12-24 11:45 ` [PATCH v4 04/10] sunrpc: remove unused bc_up operation from rpc_xprt_ops Vasily Averin
2018-12-24 11:45 ` [PATCH v4 05/10] sunrpc: remove svc_tcp_bc_class Vasily Averin
2018-12-24 11:45 ` [PATCH v4 06/10] sunrpc: remove svc_rdma_bc_class Vasily Averin
2018-12-24 11:46 ` [PATCH v4 07/10] sunrpc: remove unused xpo_prep_reply_hdr callback Vasily Averin
2018-12-24 11:46 ` [PATCH v4 08/10] sunrpc: make visible processing error in bc_svc_process() Vasily Averin
2018-12-24 11:46 ` [PATCH v4 09/10] sunrpc: fix debug message in svc_create_xprt() Vasily Averin
2018-12-24 11:46 ` [PATCH v4 10/10] nfs: minor typo in nfs4_callback_up_net() Vasily Averin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).