SUNRPC: Ensure xdr_buf_read_netobj() checks for memory overruns

Also clean up the code...

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

1 files changed, 26 insertions, 33 deletions

diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index ebdff6beba31..5a6485946f3c 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -773,44 +773,37 @@ xdr_encode_word(struct xdr_buf *buf, unsigned int base, u32 obj)
  * entirely in the head or the tail, set object to point to it; otherwise
  * try to find space for it at the end of the tail, copy it there, and
  * set obj to point to it. */
-int
-xdr_buf_read_netobj(struct xdr_buf *buf, struct xdr_netobj *obj, unsigned int offset)
+int xdr_buf_read_netobj(struct xdr_buf *buf, struct xdr_netobj *obj, unsigned int offset)
 {
-        unsigned int tail_offset = buf->head[0].iov_len + buf->page_len;
-        unsigned int obj_end_offset;
+        struct xdr_buf subbuf;
 
         if (xdr_decode_word(buf, offset, &obj->len))
-                goto out;
-        obj_end_offset = offset + 4 + obj->len;
-
-        if (obj_end_offset <= buf->head[0].iov_len) {
-                /* The obj is contained entirely in the head: */
-                obj->data = buf->head[0].iov_base + offset + 4;
-        } else if (offset + 4 >= tail_offset) {
-                if (obj_end_offset - tail_offset
-                                > buf->tail[0].iov_len)
-                        goto out;
-                /* The obj is contained entirely in the tail: */
-                obj->data = buf->tail[0].iov_base
-                        + offset - tail_offset + 4;
-        } else {
-                /* use end of tail as storage for obj:
-                 * (We don't copy to the beginning because then we'd have
-                 * to worry about doing a potentially overlapping copy.
-                 * This assumes the object is at most half the length of the
-                 * tail.) */
-                if (obj->len > buf->tail[0].iov_len)
-                        goto out;
-                obj->data = buf->tail[0].iov_base + buf->tail[0].iov_len - 
-                                obj->len;
-                if (read_bytes_from_xdr_buf(buf, offset + 4,
-                                        obj->data, obj->len))
-                        goto out;
+                return -EFAULT;
+        if (xdr_buf_subsegment(buf, &subbuf, offset + 4, obj->len))
+                return -EFAULT;
 
-        }
+        /* Is the obj contained entirely in the head? */
+        obj->data = subbuf.head[0].iov_base;
+        if (subbuf.head[0].iov_len == obj->len)
+                return 0;
+        /* ..or is the obj contained entirely in the tail? */
+        obj->data = subbuf.tail[0].iov_base;
+        if (subbuf.tail[0].iov_len == obj->len)
+                return 0;
+
+        /* use end of tail as storage for obj:
+         * (We don't copy to the beginning because then we'd have
+         * to worry about doing a potentially overlapping copy.
+         * This assumes the object is at most half the length of the
+         * tail.) */
+        if (obj->len > buf->buflen - buf->len)
+                return -ENOMEM;
+        if (buf->tail[0].iov_len != 0)
+                obj->data = buf->tail[0].iov_base + buf->tail[0].iov_len;
+        else
+                obj->data = buf->head[0].iov_base + buf->head[0].iov_len;
+        __read_bytes_from_xdr_buf(&subbuf, obj->data, obj->len);
         return 0;
-out:
-        return -1;
 }
 
 /* Returns 0 on success, or else a negative error code. */