NFS: Fix rpcrdma_inline_fixup() crash with new LISTXATTRS operation

By switching to an XFS-backed export, I am able to reproduce the
ibcomp worker crash on my client with xfstests generic/013.

For the failing LISTXATTRS operation, xdr_inline_pages() is called
with page_len=12 and buflen=128.

- When ->send_request() is called, rpcrdma_marshal_req() does not
  set up a Reply chunk because buflen is smaller than the inline
  threshold. Thus rpcrdma_convert_iovs() does not get invoked at
  all and the transport's XDRBUF_SPARSE_PAGES logic is not invoked
  on the receive buffer.

- During reply processing, rpcrdma_inline_fixup() tries to copy
  received data into rq_rcv_buf->pages because page_len is positive.
  But there are no receive pages because rpcrdma_marshal_req() never
  allocated them.

The result is that the ibcomp worker faults and dies. Sometimes that
causes a visible crash, and sometimes it results in a transport hang
without other symptoms.

RPC/RDMA's XDRBUF_SPARSE_PAGES support is not entirely correct, and
should eventually be fixed or replaced. However, my preference is
that upper-layer operations should explicitly allocate their receive
buffers (using GFP_KERNEL) when possible, rather than relying on
XDRBUF_SPARSE_PAGES.

Reported-by: Olga kornievskaia <kolga@netapp.com>
Suggested-by: Olga kornievskaia <kolga@netapp.com>
Fixes: c426fc53045e ("NFSv4.2: add the extended attribute proc functions.")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Reviewed-by: Olga kornievskaia <kolga@netapp.com>
Reviewed-by: Frank van der Linden <fllinden@amazon.com>
Tested-by: Olga kornievskaia <kolga@netapp.com>
Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 2b2211d1234e29f30ba23d0a29e74c22f6e03ea7..4fc61e3d098da631400e3d84206bbab50de490e3 100644 (file)
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -1241,12 +1241,13 @@ static ssize_t _nfs42_proc_listxattrs(struct inode *inode, void *buf,
                .rpc_resp       = &res,
        };
        u32 xdrlen;
-       int ret, np;
+       int ret, np, i;
 
 
+       ret = -ENOMEM;
        res.scratch = alloc_page(GFP_KERNEL);
        if (!res.scratch)
-               return -ENOMEM;
+               goto out;
 
        xdrlen = nfs42_listxattr_xdrsize(buflen);
        if (xdrlen > server->lxasize)
@@ -1254,9 +1255,12 @@ static ssize_t _nfs42_proc_listxattrs(struct inode *inode, void *buf,
        np = xdrlen / PAGE_SIZE + 1;
 
        pages = kcalloc(np, sizeof(struct page *), GFP_KERNEL);
-       if (pages == NULL) {
-               __free_page(res.scratch);
-               return -ENOMEM;
+       if (!pages)
+               goto out_free_scratch;
+       for (i = 0; i < np; i++) {
+               pages[i] = alloc_page(GFP_KERNEL);
+               if (!pages[i])
+                       goto out_free_pages;
        }
 
        arg.xattr_pages = pages;
@@ -1271,14 +1275,15 @@ static ssize_t _nfs42_proc_listxattrs(struct inode *inode, void *buf,
                *eofp = res.eof;
        }
 
+out_free_pages:
        while (--np >= 0) {
                if (pages[np])
                        __free_page(pages[np]);
        }
-
-       __free_page(res.scratch);
        kfree(pages);
-
+out_free_scratch:
+       __free_page(res.scratch);
+out:
        return ret;
 
 }

diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
index 6e060a88f98c2ed1df387d6ad7288d4cf89575d3..8432bd6b95f08ddc46c5a810d2031761d14987e2 100644 (file)
--- a/fs/nfs/nfs42xdr.c
+++ b/fs/nfs/nfs42xdr.c
@@ -1528,7 +1528,6 @@ static void nfs4_xdr_enc_listxattrs(struct rpc_rqst *req,
 
        rpc_prepare_reply_pages(req, args->xattr_pages, 0, args->count,
            hdr.replen);
-       req->rq_rcv_buf.flags |= XDRBUF_SPARSE_PAGES;
 
        encode_nops(&hdr);
 }