[3.19.y-ckt stable] Patch "nfs4: limit callback decoding to received bytes" has been added to staging queue

[Benjamin Coddington]

Hi Kamal,

This will break the NFS client's callback channel without Trond's fix
756b9b37cfb2e3dc76b2e43a8c097402ac736e07.  It shouldn't be added without
that one as well.

Ben

On Tue, 15 Dec 2015, Kamal Mostafa wrote:

> This is a note to let you know that I have just added a patch titled
>
>     nfs4: limit callback decoding to received bytes
>
> to the linux-3.19.y-queue branch of the 3.19.y-ckt extended stable tree
> which can be found at:
>
>     http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-3.19.y-queue
>
> This patch is scheduled to be released in version 3.19.8-ckt12.
>
> If you, or anyone else, feels it should not be added to this tree, please
> reply to this email.
>
> For more information about the 3.19.y-ckt tree, see
> https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable
>
> Thanks.
> -Kamal
>
> ------
>
> From 44e003f516dc4407fee82e3f668369a551284ead Mon Sep 17 00:00:00 2001
> From: Benjamin Coddington <bcodding at redhat.com>
> Date: Fri, 20 Nov 2015 09:55:30 -0500
> Subject: nfs4: limit callback decoding to received bytes
>
> commit 38b7631fbe42e6e247e9fc9879f961b14a687e3b upstream.
>
> A truncated cb_compound request will cause the client to decode null or
> data from a previous callback for nfs4.1 backchannel case, or uninitialized
> data for the nfs4.0 case. This is because the path through
> svc_process_common() advances the request's iov_base and decrements iov_len
> without adjusting the overall xdr_buf's len field.  That causes
> xdr_init_decode() to set up the xdr_stream with an incorrect length in
> nfs4_callback_compound().
>
> Fixing this for the nfs4.1 backchannel case first requires setting the
> correct iov_len and page_len based on the length of received data in the
> same manner as the nfs4.0 case.
>
> Then the request's xdr_buf length can be adjusted for both cases based upon
> the remaining iov_len and page_len.
>
> Signed-off-by: Benjamin Coddington <bcodding at redhat.com>
> Signed-off-by: Trond Myklebust <trond.myklebust at primarydata.com>
> Signed-off-by: Kamal Mostafa <kamal at canonical.com>
> ---
>  fs/nfs/callback_xdr.c         | 7 +++++--
>  net/sunrpc/backchannel_rqst.c | 8 ++++++++
>  net/sunrpc/svc.c              | 1 +
>  3 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
> index 02f8d09..1e36635 100644
> --- a/fs/nfs/callback_xdr.c
> +++ b/fs/nfs/callback_xdr.c
> @@ -76,7 +76,8 @@ static __be32 *read_buf(struct xdr_stream *xdr, int nbytes)
>
>  	p = xdr_inline_decode(xdr, nbytes);
>  	if (unlikely(p == NULL))
> -		printk(KERN_WARNING "NFS: NFSv4 callback reply buffer overflowed!\n");
> +		printk(KERN_WARNING "NFS: NFSv4 callback reply buffer overflowed "
> +							"or truncated request.\n");
>  	return p;
>  }
>
> @@ -892,6 +893,7 @@ static __be32 nfs4_callback_compound(struct svc_rqst *rqstp, void *argp, void *r
>  	struct cb_compound_hdr_arg hdr_arg = { 0 };
>  	struct cb_compound_hdr_res hdr_res = { NULL };
>  	struct xdr_stream xdr_in, xdr_out;
> +	struct xdr_buf *rq_arg = &rqstp->rq_arg;
>  	__be32 *p, status;
>  	struct cb_process_state cps = {
>  		.drc_status = 0,
> @@ -903,7 +905,8 @@ static __be32 nfs4_callback_compound(struct svc_rqst *rqstp, void *argp, void *r
>
>  	dprintk("%s: start\n", __func__);
>
> -	xdr_init_decode(&xdr_in, &rqstp->rq_arg, rqstp->rq_arg.head[0].iov_base);
> +	rq_arg->len = rq_arg->head[0].iov_len + rq_arg->page_len;
> +	xdr_init_decode(&xdr_in, rq_arg, rq_arg->head[0].iov_base);
>
>  	p = (__be32*)((char *)rqstp->rq_res.head[0].iov_base + rqstp->rq_res.head[0].iov_len);
>  	xdr_init_encode(&xdr_out, &rqstp->rq_res, p);
> diff --git a/net/sunrpc/backchannel_rqst.c b/net/sunrpc/backchannel_rqst.c
> index 28504df..b57cee1 100644
> --- a/net/sunrpc/backchannel_rqst.c
> +++ b/net/sunrpc/backchannel_rqst.c
> @@ -308,11 +308,19 @@ void xprt_complete_bc_request(struct rpc_rqst *req, uint32_t copied)
>  {
>  	struct rpc_xprt *xprt = req->rq_xprt;
>  	struct svc_serv *bc_serv = xprt->bc_serv;
> +	struct xdr_buf *rq_rcv_buf = &req->rq_rcv_buf;
>
>  	spin_lock(&xprt->bc_pa_lock);
>  	list_del(&req->rq_bc_pa_list);
>  	spin_unlock(&xprt->bc_pa_lock);
>
> +	if (copied <= rq_rcv_buf->head[0].iov_len) {
> +		rq_rcv_buf->head[0].iov_len = copied;
> +		rq_rcv_buf->page_len = 0;
> +	} else {
> +		rq_rcv_buf->page_len = copied - rq_rcv_buf->head[0].iov_len;
> +	}
> +
>  	req->rq_private_buf.len = copied;
>  	set_bit(RPC_BC_PA_IN_USE, &req->rq_bc_pa_state);
>
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 91eaef1..b59b6a7 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -1361,6 +1361,7 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
>  	memcpy(&rqstp->rq_addr, &req->rq_xprt->addr, rqstp->rq_addrlen);
>  	memcpy(&rqstp->rq_arg, &req->rq_rcv_buf, sizeof(rqstp->rq_arg));
>  	memcpy(&rqstp->rq_res, &req->rq_snd_buf, sizeof(rqstp->rq_res));
> +	rqstp->rq_arg.len = req->rq_private_buf.len;
>
>  	/* reset result send buffer "put" position */
>  	resv->iov_len = 0;
> --
> 1.9.1
>
>