Re: Patch "nfs4: limit callback decoding to received bytes" has been added to the 4.3-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Greg,

On Fri, Dec 11, 2015 at 12:20 PM,  <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> This is a note to let you know that I've just added the patch titled
>
>     nfs4: limit callback decoding to received bytes
>
> to the 4.3-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
> The filename of the patch is:
>      nfs4-limit-callback-decoding-to-received-bytes.patch
> and it can be found in the queue-4.3 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@xxxxxxxxxxxxxxx> know about it.
>
>
> From 38b7631fbe42e6e247e9fc9879f961b14a687e3b Mon Sep 17 00:00:00 2001
> From: Benjamin Coddington <bcodding@xxxxxxxxxx>
> Date: Fri, 20 Nov 2015 09:55:30 -0500
> Subject: nfs4: limit callback decoding to received bytes
>
> From: Benjamin Coddington <bcodding@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>
> ---
>  fs/nfs/callback_xdr.c         |    7 +++++--
>  net/sunrpc/backchannel_rqst.c |    8 ++++++++
>  net/sunrpc/svc.c              |    1 +
>  3 files changed, 14 insertions(+), 2 deletions(-)
>
> --- a/fs/nfs/callback_xdr.c
> +++ b/fs/nfs/callback_xdr.c
> @@ -76,7 +76,8 @@ static __be32 *read_buf(struct xdr_strea
>
>         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(str
>         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(str
>
>         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);
> --- a/net/sunrpc/backchannel_rqst.c
> +++ b/net/sunrpc/backchannel_rqst.c
> @@ -333,12 +333,20 @@ void xprt_complete_bc_request(struct rpc
>  {
>         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);
>         xprt_dec_alloc_count(xprt, 1);
>         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);
>
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -1363,6 +1363,7 @@ bc_svc_process(struct svc_serv *serv, st
>         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;
>

This patch turned out to cause some issues with the NFS callback
channel. I have a fix, but it hasn't been upstreamed yet. Can we
therefore please hold on this patch for now, and remove it from all
the stable queues. I'll resubmit when the fix is in mainline.

Thanks
  Trond
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]