diff options
author | Vasily Averin <vvs@virtuozzo.com> | 2018-12-24 06:44:52 -0500 |
---|---|---|
committer | J. Bruce Fields <bfields@redhat.com> | 2018-12-27 21:00:58 -0500 |
commit | d4b09acf924b84bae77cad090a9d108e70b43643 (patch) | |
tree | a5d2cc482f5aadfa509c83d61abf883366feb787 | |
parent | b8be5674fa9a6f3677865ea93f7803c4212f3e10 (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.h | 5 | ||||
-rw-r--r-- | include/trace/events/sunrpc.h | 6 | ||||
-rw-r--r-- | net/sunrpc/svc.c | 9 | ||||
-rw-r--r-- | net/sunrpc/svc_xprt.c | 5 | ||||
-rw-r--r-- | 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 { | |||
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 | */ |
469 | void svc_reserve(struct svc_rqst *rqstp, int space) | 469 | void 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 | */ |
1176 | static void svc_tcp_prep_reply_hdr(struct svc_rqst *rqstp) | 1176 | void 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 | ||