[Devel] [PATCH RHEL7 COMMIT] ms/sunrpc: use-after-free in svc_process_common()

Konstantin Khorenko khorenko at virtuozzo.com
Tue Jan 15 14:38:45 MSK 2019


The commit is pushed to "branch-rh7-3.10.0-957.1.3.vz7.83.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-957.1.3.vz7.83.6
------>
commit c27b4e9d65f2728f11c5cf311a9fc9795c4c26aa
Author: Vasily Averin <vvs at virtuozzo.com>
Date:   Tue Jan 15 14:38:43 2019 +0300

    ms/sunrpc: use-after-free in svc_process_common()
    
    Backported mainline commit d4b09acf924b84bae77cad090a9d108e70b43643
    
        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 <vvs at virtuozzo.com>
        Cc: stable at vger.kernel.org
        Fixes: 23c20ecd4475 ("NFS: callback up - users counting cleanup")
        Signed-off-by: J. Bruce Fields <bfields at redhat.com>
    
    v2: - added lost extern svc_tcp_prep_reply_hdr()
        - dropped trace_svc_process() changes
        - context fixes in svc_process_common()
    
    https://jira.sw.ru/browse/PSBM-89609
    Signed-off-by: Vasily Averin <vvs at virtuozzo.com>
---
 include/linux/sunrpc/svc.h |  5 ++++-
 net/sunrpc/svc.c           | 10 +++++++---
 net/sunrpc/svc_xprt.c      |  5 +++--
 net/sunrpc/svcsock.c       |  2 +-
 4 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 2b3086893194..882fb9086184 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -291,9 +291,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/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 99968dec80cc..9d662e64841c 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1063,6 +1063,8 @@ 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
 
+extern void svc_tcp_prep_reply_hdr(struct svc_rqst *);
+
 /*
  * Common routine for processing the RPC request.
  */
@@ -1092,7 +1094,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);
 
@@ -1139,7 +1142,8 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	case SVC_DENIED:
 		goto err_bad_auth;
 	case SVC_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);
 	case SVC_DROP:
 		goto dropit;
@@ -1360,10 +1364,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 7bfe1fb42add..2f30373a9b50 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -510,10 +510,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 e8c3277a3816..7650a70a2acf 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1208,7 +1208,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];
 



More information about the Devel mailing list