Commit 283a1966 authored by Vasily Averin's avatar Vasily Averin Committed by Stefan Bader

sunrpc: use-after-free in svc_process_common()

BugLink: https://bugs.launchpad.net/bugs/1818237

commit d4b09acf 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: default avatarVasily Averin <vvs@virtuozzo.com>
Cc: stable@vger.kernel.org
Fixes: 23c20ecd ("NFS: callback up - users counting cleanup")
Signed-off-by: default avatarJ. Bruce Fields <bfields@redhat.com>
v2: - added lost extern svc_tcp_prep_reply_hdr()
    - dropped trace_svc_process() changes
    - context fixes in svc_process_common()
Signed-off-by: default avatarVasily Averin <vvs@virtuozzo.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarJuerg Haefliger <juergh@canonical.com>
Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
parent 07b20cf7
...@@ -290,9 +290,12 @@ struct svc_rqst { ...@@ -290,9 +290,12 @@ struct svc_rqst {
struct svc_cacherep * rq_cacherep; /* cache info */ struct svc_cacherep * rq_cacherep; /* cache info */
struct task_struct *rq_task; /* service thread */ struct task_struct *rq_task; /* service thread */
spinlock_t rq_lock; /* per-request lock */ 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 * Rigorous type checking on sockaddr type conversions
......
...@@ -1062,6 +1062,8 @@ void svc_printk(struct svc_rqst *rqstp, const char *fmt, ...) ...@@ -1062,6 +1062,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, ...) {} static __printf(2,3) void svc_printk(struct svc_rqst *rqstp, const char *fmt, ...) {}
#endif #endif
extern void svc_tcp_prep_reply_hdr(struct svc_rqst *);
/* /*
* Common routine for processing the RPC request. * Common routine for processing the RPC request.
*/ */
...@@ -1091,7 +1093,8 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv) ...@@ -1091,7 +1093,8 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
clear_bit(RQ_DROPME, &rqstp->rq_flags); clear_bit(RQ_DROPME, &rqstp->rq_flags);
/* Setup reply header */ /* 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); svc_putu32(resv, rqstp->rq_xid);
...@@ -1138,7 +1141,8 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv) ...@@ -1138,7 +1141,8 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
case SVC_DENIED: case SVC_DENIED:
goto err_bad_auth; goto err_bad_auth;
case SVC_CLOSE: 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); svc_close_xprt(rqstp->rq_xprt);
case SVC_DROP: case SVC_DROP:
goto dropit; goto dropit;
...@@ -1360,10 +1364,10 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req, ...@@ -1360,10 +1364,10 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
dprintk("svc: %s(%p)\n", __func__, req); dprintk("svc: %s(%p)\n", __func__, req);
/* Build the svc_rqst used by the common processing routine */ /* 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_xid = req->rq_xid;
rqstp->rq_prot = req->rq_xprt->prot; rqstp->rq_prot = req->rq_xprt->prot;
rqstp->rq_server = serv; rqstp->rq_server = serv;
rqstp->rq_bc_net = req->rq_xprt->xprt_net;
rqstp->rq_addrlen = sizeof(req->rq_xprt->addr); rqstp->rq_addrlen = sizeof(req->rq_xprt->addr);
memcpy(&rqstp->rq_addr, &req->rq_xprt->addr, rqstp->rq_addrlen); memcpy(&rqstp->rq_addr, &req->rq_xprt->addr, rqstp->rq_addrlen);
......
...@@ -454,10 +454,11 @@ static struct svc_xprt *svc_xprt_dequeue(struct svc_pool *pool) ...@@ -454,10 +454,11 @@ static struct svc_xprt *svc_xprt_dequeue(struct svc_pool *pool)
*/ */
void svc_reserve(struct svc_rqst *rqstp, int space) void svc_reserve(struct svc_rqst *rqstp, int space)
{ {
struct svc_xprt *xprt = rqstp->rq_xprt;
space += rqstp->rq_res.head[0].iov_len; space += rqstp->rq_res.head[0].iov_len;
if (space < rqstp->rq_reserved) { if (xprt && space < rqstp->rq_reserved) {
struct svc_xprt *xprt = rqstp->rq_xprt;
atomic_sub((rqstp->rq_reserved - space), &xprt->xpt_reserved); atomic_sub((rqstp->rq_reserved - space), &xprt->xpt_reserved);
rqstp->rq_reserved = space; rqstp->rq_reserved = space;
......
...@@ -1240,7 +1240,7 @@ static int svc_tcp_sendto(struct svc_rqst *rqstp) ...@@ -1240,7 +1240,7 @@ static int svc_tcp_sendto(struct svc_rqst *rqstp)
/* /*
* Setup response header. TCP has a 4B record length field. * 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]; struct kvec *resv = &rqstp->rq_res.head[0];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment