* [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).