SUNRPC: Bound-check xdr_buf_to_bvec() stores before writing

xdr_buf_to_bvec() writes a bio_vec into the caller's array before
testing whether that slot is in range, and the head branch performs
the store with no check at all. When the caller's budget is exactly
used up, the next store lands one element past the end of the array.
The overflow label returns count - 1, which masks the surplus store
but cannot undo it.

rq_bvec, the array passed by nfsd_vfs_write(), is allocated to
exactly rq_maxpages entries with no slack. The OOB store can land in
adjacent slab memory; the bv_len and bv_offset fields written there
are derived from client-supplied RPC payload sizes.

Move the in-range check ahead of the store in the head, page-loop,
and tail branches. With the check at the top of each sequence, count
is incremented only after a successful store, so the overflow label
can return count directly.

Reported-by: Chris Mason <clm@meta.com>
Fixes: 2eb2b9358181 ("SUNRPC: Convert svc_tcp_sendmsg to use bio_vecs directly")
Cc: stable@vger.kernel.org
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index 6bd588dfbfc0da8ce18cf244beca310972bd2469..8f52782d8a3727e6106f384cbfb0c7d5a68b0594 100644 (file)
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -152,6 +152,8 @@ unsigned int xdr_buf_to_bvec(struct bio_vec *bvec, unsigned int bvec_size,
        unsigned int count = 0;
 
        if (head->iov_len) {
+               if (unlikely(count >= bvec_size))
+                       goto bvec_overflow;
                bvec_set_virt(bvec++, head->iov_base, head->iov_len);
                ++count;
        }
@@ -165,25 +167,27 @@ unsigned int xdr_buf_to_bvec(struct bio_vec *bvec, unsigned int bvec_size,
                while (remaining > 0) {
                        len = min_t(unsigned int, remaining,
                                    PAGE_SIZE - offset);
+                       if (unlikely(count >= bvec_size))
+                               goto bvec_overflow;
                        bvec_set_page(bvec++, *pages++, len, offset);
                        remaining -= len;
                        offset = 0;
-                       if (unlikely(++count > bvec_size))
-                               goto bvec_overflow;
+                       ++count;
                }
        }
 
        if (tail->iov_len) {
-               bvec_set_virt(bvec, tail->iov_base, tail->iov_len);
-               if (unlikely(++count > bvec_size))
+               if (unlikely(count >= bvec_size))
                        goto bvec_overflow;
+               bvec_set_virt(bvec, tail->iov_base, tail->iov_len);
+               ++count;
        }
 
        return count;
 
 bvec_overflow:
        pr_warn_once("%s: bio_vec array overflow\n", __func__);
-       return count - 1;
+       return count;
 }
 EXPORT_SYMBOL_GPL(xdr_buf_to_bvec);