summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVasily Averin <vvs@virtuozzo.com>2018-12-24 06:44:52 -0500
committerJ. Bruce Fields <bfields@redhat.com>2018-12-27 21:00:58 -0500
commitd4b09acf924b84bae77cad090a9d108e70b43643 (patch)
treea5d2cc482f5aadfa509c83d61abf883366feb787
parentb8be5674fa9a6f3677865ea93f7803c4212f3e10 (diff)
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@virtuozzo.com> Cc: stable@vger.kernel.org Fixes: 23c20ecd4475 ("NFS: callback up - users counting cleanup") Signed-off-by: J. Bruce Fields <bfields@redhat.com>
-rw-r--r--include/linux/sunrpc/svc.h5
-rw-r--r--include/trace/events/sunrpc.h6
-rw-r--r--net/sunrpc/svc.c9
-rw-r--r--net/sunrpc/svc_xprt.c5
-rw-r--r--net/sunrpc/svcsock.c2
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 {
295 struct svc_cacherep * rq_cacherep; /* cache info */ 295 struct svc_cacherep * rq_cacherep; /* cache info */
296 struct task_struct *rq_task; /* service thread */ 296 struct task_struct *rq_task; /* service thread */
297 spinlock_t rq_lock; /* per-request lock */ 297 spinlock_t rq_lock; /* per-request lock */
298 struct net *rq_bc_net; /* pointer to backchannel's
299 * net namespace
300 */
298}; 301};
299 302
300#define SVC_NET(svc_rqst) (svc_rqst->rq_xprt->xpt_net) 303#define SVC_NET(rqst) (rqst->rq_xprt ? rqst->rq_xprt->xpt_net : rqst->rq_bc_net)
301 304
302/* 305/*
303 * Rigorous type checking on sockaddr type conversions 306 * 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,
569 __field(u32, vers) 569 __field(u32, vers)
570 __field(u32, proc) 570 __field(u32, proc)
571 __string(service, name) 571 __string(service, name)
572 __string(addr, rqst->rq_xprt->xpt_remotebuf) 572 __string(addr, rqst->rq_xprt ?
573 rqst->rq_xprt->xpt_remotebuf : "(null)")
573 ), 574 ),
574 575
575 TP_fast_assign( 576 TP_fast_assign(
@@ -577,7 +578,8 @@ TRACE_EVENT(svc_process,
577 __entry->vers = rqst->rq_vers; 578 __entry->vers = rqst->rq_vers;
578 __entry->proc = rqst->rq_proc; 579 __entry->proc = rqst->rq_proc;
579 __assign_str(service, name); 580 __assign_str(service, name);
580 __assign_str(addr, rqst->rq_xprt->xpt_remotebuf); 581 __assign_str(addr, rqst->rq_xprt ?
582 rqst->rq_xprt->xpt_remotebuf : "(null)");
581 ), 583 ),
582 584
583 TP_printk("addr=%s xid=0x%08x service=%s vers=%u proc=%u", 585 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)
1172 clear_bit(RQ_DROPME, &rqstp->rq_flags); 1172 clear_bit(RQ_DROPME, &rqstp->rq_flags);
1173 1173
1174 /* Setup reply header */ 1174 /* Setup reply header */
1175 rqstp->rq_xprt->xpt_ops->xpo_prep_reply_hdr(rqstp); 1175 if (rqstp->rq_prot == IPPROTO_TCP)
1176 svc_tcp_prep_reply_hdr(rqstp);
1176 1177
1177 svc_putu32(resv, rqstp->rq_xid); 1178 svc_putu32(resv, rqstp->rq_xid);
1178 1179
@@ -1244,7 +1245,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
1244 * for lower versions. RPC_PROG_MISMATCH seems to be the closest 1245 * for lower versions. RPC_PROG_MISMATCH seems to be the closest
1245 * fit. 1246 * fit.
1246 */ 1247 */
1247 if (versp->vs_need_cong_ctrl && 1248 if (versp->vs_need_cong_ctrl && rqstp->rq_xprt &&
1248 !test_bit(XPT_CONG_CTRL, &rqstp->rq_xprt->xpt_flags)) 1249 !test_bit(XPT_CONG_CTRL, &rqstp->rq_xprt->xpt_flags))
1249 goto err_bad_vers; 1250 goto err_bad_vers;
1250 1251
@@ -1336,7 +1337,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
1336 return 0; 1337 return 0;
1337 1338
1338 close: 1339 close:
1339 if (test_bit(XPT_TEMP, &rqstp->rq_xprt->xpt_flags)) 1340 if (rqstp->rq_xprt && test_bit(XPT_TEMP, &rqstp->rq_xprt->xpt_flags))
1340 svc_close_xprt(rqstp->rq_xprt); 1341 svc_close_xprt(rqstp->rq_xprt);
1341 dprintk("svc: svc_process close\n"); 1342 dprintk("svc: svc_process close\n");
1342 return 0; 1343 return 0;
@@ -1459,10 +1460,10 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
1459 dprintk("svc: %s(%p)\n", __func__, req); 1460 dprintk("svc: %s(%p)\n", __func__, req);
1460 1461
1461 /* Build the svc_rqst used by the common processing routine */ 1462 /* Build the svc_rqst used by the common processing routine */
1462 rqstp->rq_xprt = serv->sv_bc_xprt;
1463 rqstp->rq_xid = req->rq_xid; 1463 rqstp->rq_xid = req->rq_xid;
1464 rqstp->rq_prot = req->rq_xprt->prot; 1464 rqstp->rq_prot = req->rq_xprt->prot;
1465 rqstp->rq_server = serv; 1465 rqstp->rq_server = serv;
1466 rqstp->rq_bc_net = req->rq_xprt->xprt_net;
1466 1467
1467 rqstp->rq_addrlen = sizeof(req->rq_xprt->addr); 1468 rqstp->rq_addrlen = sizeof(req->rq_xprt->addr);
1468 memcpy(&rqstp->rq_addr, &req->rq_xprt->addr, rqstp->rq_addrlen); 1469 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 @@ out:
468 */ 468 */
469void svc_reserve(struct svc_rqst *rqstp, int space) 469void svc_reserve(struct svc_rqst *rqstp, int space)
470{ 470{
471 struct svc_xprt *xprt = rqstp->rq_xprt;
472
471 space += rqstp->rq_res.head[0].iov_len; 473 space += rqstp->rq_res.head[0].iov_len;
472 474
473 if (space < rqstp->rq_reserved) { 475 if (xprt && space < rqstp->rq_reserved) {
474 struct svc_xprt *xprt = rqstp->rq_xprt;
475 atomic_sub((rqstp->rq_reserved - space), &xprt->xpt_reserved); 476 atomic_sub((rqstp->rq_reserved - space), &xprt->xpt_reserved);
476 rqstp->rq_reserved = space; 477 rqstp->rq_reserved = space;
477 478
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)
1173/* 1173/*
1174 * Setup response header. TCP has a 4B record length field. 1174 * Setup response header. TCP has a 4B record length field.
1175 */ 1175 */
1176static void svc_tcp_prep_reply_hdr(struct svc_rqst *rqstp) 1176void svc_tcp_prep_reply_hdr(struct svc_rqst *rqstp)
1177{ 1177{
1178 struct kvec *resv = &rqstp->rq_res.head[0]; 1178 struct kvec *resv = &rqstp->rq_res.head[0];
1179 1179