Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 37c791a0 authored by Vasily Averin's avatar Vasily Averin Committed by Greg Kroah-Hartman
Browse files

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: 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>
parent c173c384

include/linux/sunrpc/svc.h

+4 −1
Original line number Diff line number Diff line
@@ -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

net/sunrpc/svc.c

+6 −3
Original line number Diff line number Diff line
@@ -1137,6 +1137,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.

 */
@@ -1166,7 +1168,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);
@@ -1312,7 +1315,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;
@@ -1439,10 +1442,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);

net/sunrpc/svc_xprt.c

+3 −2
Original line number Diff line number Diff line
@@ -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;

net/sunrpc/svcsock.c

+1 −1
Original line number Diff line number Diff line
@@ -1195,7 +1195,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];