From 97bce63408f192712574a4d9d6dcab794eed3a79 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Tue, 27 Nov 2018 11:11:35 -0500 Subject: svcrdma: Optimize the logic that selects the R_key to invalidate o Select the R_key to invalidate while the CPU cache still contains the received RPC Call transport header, rather than waiting until we're about to send the RPC Reply. o Choose Send With Invalidate if there is exactly one distinct R_key in the received transport header. If there's more than one, the client will have to perform local invalidation after it has already waited for remote invalidation. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- include/linux/sunrpc/svc_rdma.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include/linux/sunrpc') diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h index e6e26918504c..7e22681333d0 100644 --- a/include/linux/sunrpc/svc_rdma.h +++ b/include/linux/sunrpc/svc_rdma.h @@ -135,6 +135,7 @@ struct svc_rdma_recv_ctxt { u32 rc_byte_len; unsigned int rc_page_count; unsigned int rc_hdr_count; + u32 rc_inv_rkey; struct page *rc_pages[RPCSVC_MAXPAGES]; }; -- cgit v1.2.3 From d4b09acf924b84bae77cad090a9d108e70b43643 Mon Sep 17 00:00:00 2001 From: Vasily Averin Date: Mon, 24 Dec 2018 14:44:52 +0300 Subject: sunrpc: use-after-free in svc_process_common() 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 Cc: stable@vger.kernel.org Fixes: 23c20ecd4475 ("NFS: callback up - users counting cleanup") Signed-off-by: J. Bruce Fields --- include/linux/sunrpc/svc.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'include/linux/sunrpc') 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 -- cgit v1.2.3 From a289ce5311f406bf846614591300a948ebc42062 Mon Sep 17 00:00:00 2001 From: Vasily Averin Date: Mon, 24 Dec 2018 14:45:04 +0300 Subject: sunrpc: replace svc_serv->sv_bc_xprt by boolean flag 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 Signed-off-by: J. Bruce Fields --- include/linux/sunrpc/bc_xprt.h | 10 ++++------ include/linux/sunrpc/svc.h | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) (limited to 'include/linux/sunrpc') 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 */ }; -- cgit v1.2.3 From 4aa5cffefa6f8af8f16490df58b8f0d827911b58 Mon Sep 17 00:00:00 2001 From: Vasily Averin Date: Mon, 24 Dec 2018 14:45:25 +0300 Subject: sunrpc: remove unused bc_up operation from rpc_xprt_ops Signed-off-by: Vasily Averin Signed-off-by: J. Bruce Fields --- include/linux/sunrpc/xprt.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include/linux/sunrpc') 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, -- cgit v1.2.3 From 64e20ba204df539a76004114e08abf1156302e35 Mon Sep 17 00:00:00 2001 From: Vasily Averin Date: Mon, 24 Dec 2018 14:46:00 +0300 Subject: sunrpc: remove unused xpo_prep_reply_hdr callback 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 Signed-off-by: J. Bruce Fields --- include/linux/sunrpc/svc_rdma.h | 1 - include/linux/sunrpc/svc_xprt.h | 1 - 2 files changed, 2 deletions(-) (limited to 'include/linux/sunrpc') diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h index 7e22681333d0..981f0d726ad4 100644 --- a/include/linux/sunrpc/svc_rdma.h +++ b/include/linux/sunrpc/svc_rdma.h @@ -193,7 +193,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 *); -- cgit v1.2.3 From 0ad30ff67bd3e82da8c1dc4d74b88aca846dbbd9 Mon Sep 17 00:00:00 2001 From: Vasily Averin Date: Sat, 29 Dec 2018 16:38:51 +0300 Subject: nfs: fixed broken compilation in nfs_callback_up_net() Patch fixes compilation error in nfs_callback_up_net() serv->sv_bc_enabled is defined under enabled CONFIG_SUNRPC_BACKCHANNEL, however nfs_callback_up_net() can access it even if this config option was not set. Fixes: a289ce5311f4 (sunrpc: replace svc_serv->sv_bc_xprt by boolean flag) Reported-by: kbuild test robot Signed-off-by: Vasily Averin Signed-off-by: J. Bruce Fields --- include/linux/sunrpc/bc_xprt.h | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'include/linux/sunrpc') diff --git a/include/linux/sunrpc/bc_xprt.h b/include/linux/sunrpc/bc_xprt.h index 4e8c773d02be..d4229a78524a 100644 --- a/include/linux/sunrpc/bc_xprt.h +++ b/include/linux/sunrpc/bc_xprt.h @@ -51,6 +51,11 @@ static inline bool svc_is_backchannel(const struct svc_rqst *rqstp) { return rqstp->rq_server->sv_bc_enabled; } + +static inline void set_bc_enabled(struct svc_serv *serv) +{ + serv->sv_bc_enabled = true; +} #else /* CONFIG_SUNRPC_BACKCHANNEL */ static inline int xprt_setup_backchannel(struct rpc_xprt *xprt, unsigned int min_reqs) @@ -63,6 +68,10 @@ static inline bool svc_is_backchannel(const struct svc_rqst *rqstp) return false; } +static inline void set_bc_enabled(struct svc_serv *serv) +{ +} + static inline void xprt_free_bc_request(struct rpc_rqst *req) { } -- cgit v1.2.3