sunrpc: use-after-free in svc_process_common()

commit d4b09acf924b84bae77cad090a9d108e70b43643 upstream.

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>
Cc: stable@vger.kernel.org
Fixes: 23c20ecd4475 ("NFS: callback up - users counting cleanup")
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
v2: added lost extern svc_tcp_prep_reply_hdr()
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 73e130a..fdb6b31 100644 (file)
--- 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 bbb08a3..a2644c4 100644 (file)
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -582,7 +582,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(
@@ -590,7 +591,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 d13e05f..d65f8d3 100644 (file)
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1144,6 +1144,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.
  */
@@ -1172,7 +1174,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 +1247,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 +1339,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 +1462,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 83ccd02..6cf0fd3 100644 (file)
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -469,10 +469,11 @@ out:
  */
 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 fc1c0d9..97a8282 100644 (file)
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1198,7 +1198,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];