Commit 64bd577e authored by Trond Myklebust's avatar Trond Myklebust

NFS: Let xdr_read_pages() check for buffer overflows

xdr_read_pages will already do all of the buffer overflow checks that are
currently being open-coded in the various callers. This patch simplifies
the existing code by replacing the open coded checks.
Signed-off-by: default avatarTrond Myklebust <Trond.Myklebust@netapp.com>
parent c337d365
...@@ -106,19 +106,16 @@ static void print_overflow_msg(const char *func, const struct xdr_stream *xdr) ...@@ -106,19 +106,16 @@ static void print_overflow_msg(const char *func, const struct xdr_stream *xdr)
static int decode_nfsdata(struct xdr_stream *xdr, struct nfs_readres *result) static int decode_nfsdata(struct xdr_stream *xdr, struct nfs_readres *result)
{ {
u32 recvd, count; u32 recvd, count;
size_t hdrlen;
__be32 *p; __be32 *p;
p = xdr_inline_decode(xdr, 4); p = xdr_inline_decode(xdr, 4);
if (unlikely(p == NULL)) if (unlikely(p == NULL))
goto out_overflow; goto out_overflow;
count = be32_to_cpup(p); count = be32_to_cpup(p);
hdrlen = (u8 *)xdr->p - (u8 *)xdr->iov->iov_base; recvd = xdr_read_pages(xdr, count);
recvd = xdr->buf->len - hdrlen;
if (unlikely(count > recvd)) if (unlikely(count > recvd))
goto out_cheating; goto out_cheating;
out: out:
xdr_read_pages(xdr, count);
result->eof = 0; /* NFSv2 does not pass EOF flag on the wire. */ result->eof = 0; /* NFSv2 does not pass EOF flag on the wire. */
result->count = count; result->count = count;
return count; return count;
...@@ -440,7 +437,6 @@ static void encode_path(struct xdr_stream *xdr, struct page **pages, u32 length) ...@@ -440,7 +437,6 @@ static void encode_path(struct xdr_stream *xdr, struct page **pages, u32 length)
static int decode_path(struct xdr_stream *xdr) static int decode_path(struct xdr_stream *xdr)
{ {
u32 length, recvd; u32 length, recvd;
size_t hdrlen;
__be32 *p; __be32 *p;
p = xdr_inline_decode(xdr, 4); p = xdr_inline_decode(xdr, 4);
...@@ -449,12 +445,9 @@ static int decode_path(struct xdr_stream *xdr) ...@@ -449,12 +445,9 @@ static int decode_path(struct xdr_stream *xdr)
length = be32_to_cpup(p); length = be32_to_cpup(p);
if (unlikely(length >= xdr->buf->page_len || length > NFS_MAXPATHLEN)) if (unlikely(length >= xdr->buf->page_len || length > NFS_MAXPATHLEN))
goto out_size; goto out_size;
hdrlen = (u8 *)xdr->p - (u8 *)xdr->iov->iov_base; recvd = xdr_read_pages(xdr, length);
recvd = xdr->buf->len - hdrlen;
if (unlikely(length > recvd)) if (unlikely(length > recvd))
goto out_cheating; goto out_cheating;
xdr_read_pages(xdr, length);
xdr_terminate_string(xdr->buf, length); xdr_terminate_string(xdr->buf, length);
return 0; return 0;
out_size: out_size:
...@@ -972,16 +965,7 @@ int nfs2_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry, ...@@ -972,16 +965,7 @@ int nfs2_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry,
*/ */
static int decode_readdirok(struct xdr_stream *xdr) static int decode_readdirok(struct xdr_stream *xdr)
{ {
u32 recvd, pglen; return xdr_read_pages(xdr, xdr->buf->page_len);
size_t hdrlen;
pglen = xdr->buf->page_len;
hdrlen = (u8 *)xdr->p - (u8 *)xdr->iov->iov_base;
recvd = xdr->buf->len - hdrlen;
if (pglen > recvd)
pglen = recvd;
xdr_read_pages(xdr, pglen);
return pglen;
} }
static int nfs2_xdr_dec_readdirres(struct rpc_rqst *req, static int nfs2_xdr_dec_readdirres(struct rpc_rqst *req,
......
...@@ -246,7 +246,6 @@ static void encode_nfspath3(struct xdr_stream *xdr, struct page **pages, ...@@ -246,7 +246,6 @@ static void encode_nfspath3(struct xdr_stream *xdr, struct page **pages,
static int decode_nfspath3(struct xdr_stream *xdr) static int decode_nfspath3(struct xdr_stream *xdr)
{ {
u32 recvd, count; u32 recvd, count;
size_t hdrlen;
__be32 *p; __be32 *p;
p = xdr_inline_decode(xdr, 4); p = xdr_inline_decode(xdr, 4);
...@@ -255,12 +254,9 @@ static int decode_nfspath3(struct xdr_stream *xdr) ...@@ -255,12 +254,9 @@ static int decode_nfspath3(struct xdr_stream *xdr)
count = be32_to_cpup(p); count = be32_to_cpup(p);
if (unlikely(count >= xdr->buf->page_len || count > NFS3_MAXPATHLEN)) if (unlikely(count >= xdr->buf->page_len || count > NFS3_MAXPATHLEN))
goto out_nametoolong; goto out_nametoolong;
hdrlen = (u8 *)xdr->p - (u8 *)xdr->iov->iov_base; recvd = xdr_read_pages(xdr, count);
recvd = xdr->buf->len - hdrlen;
if (unlikely(count > recvd)) if (unlikely(count > recvd))
goto out_cheating; goto out_cheating;
xdr_read_pages(xdr, count);
xdr_terminate_string(xdr->buf, count); xdr_terminate_string(xdr->buf, count);
return 0; return 0;
...@@ -1587,7 +1583,6 @@ static int decode_read3resok(struct xdr_stream *xdr, ...@@ -1587,7 +1583,6 @@ static int decode_read3resok(struct xdr_stream *xdr,
struct nfs_readres *result) struct nfs_readres *result)
{ {
u32 eof, count, ocount, recvd; u32 eof, count, ocount, recvd;
size_t hdrlen;
__be32 *p; __be32 *p;
p = xdr_inline_decode(xdr, 4 + 4 + 4); p = xdr_inline_decode(xdr, 4 + 4 + 4);
...@@ -1598,13 +1593,10 @@ static int decode_read3resok(struct xdr_stream *xdr, ...@@ -1598,13 +1593,10 @@ static int decode_read3resok(struct xdr_stream *xdr,
ocount = be32_to_cpup(p++); ocount = be32_to_cpup(p++);
if (unlikely(ocount != count)) if (unlikely(ocount != count))
goto out_mismatch; goto out_mismatch;
hdrlen = (u8 *)xdr->p - (u8 *)xdr->iov->iov_base; recvd = xdr_read_pages(xdr, count);
recvd = xdr->buf->len - hdrlen;
if (unlikely(count > recvd)) if (unlikely(count > recvd))
goto out_cheating; goto out_cheating;
out: out:
xdr_read_pages(xdr, count);
result->eof = eof; result->eof = eof;
result->count = count; result->count = count;
return count; return count;
...@@ -2039,16 +2031,7 @@ int nfs3_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry, ...@@ -2039,16 +2031,7 @@ int nfs3_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry,
*/ */
static int decode_dirlist3(struct xdr_stream *xdr) static int decode_dirlist3(struct xdr_stream *xdr)
{ {
u32 recvd, pglen; return xdr_read_pages(xdr, xdr->buf->page_len);
size_t hdrlen;
pglen = xdr->buf->page_len;
hdrlen = (u8 *)xdr->p - (u8 *)xdr->iov->iov_base;
recvd = xdr->buf->len - hdrlen;
if (pglen > recvd)
pglen = recvd;
xdr_read_pages(xdr, pglen);
return pglen;
} }
static int decode_readdir3resok(struct xdr_stream *xdr, static int decode_readdir3resok(struct xdr_stream *xdr,
......
...@@ -4920,9 +4920,8 @@ static int decode_putrootfh(struct xdr_stream *xdr) ...@@ -4920,9 +4920,8 @@ static int decode_putrootfh(struct xdr_stream *xdr)
static int decode_read(struct xdr_stream *xdr, struct rpc_rqst *req, struct nfs_readres *res) static int decode_read(struct xdr_stream *xdr, struct rpc_rqst *req, struct nfs_readres *res)
{ {
struct kvec *iov = req->rq_rcv_buf.head;
__be32 *p; __be32 *p;
uint32_t count, eof, recvd, hdrlen; uint32_t count, eof, recvd;
int status; int status;
status = decode_op_hdr(xdr, OP_READ); status = decode_op_hdr(xdr, OP_READ);
...@@ -4933,15 +4932,13 @@ static int decode_read(struct xdr_stream *xdr, struct rpc_rqst *req, struct nfs_ ...@@ -4933,15 +4932,13 @@ static int decode_read(struct xdr_stream *xdr, struct rpc_rqst *req, struct nfs_
goto out_overflow; goto out_overflow;
eof = be32_to_cpup(p++); eof = be32_to_cpup(p++);
count = be32_to_cpup(p); count = be32_to_cpup(p);
hdrlen = (u8 *) xdr->p - (u8 *) iov->iov_base; recvd = xdr_read_pages(xdr, count);
recvd = req->rq_rcv_buf.len - hdrlen;
if (count > recvd) { if (count > recvd) {
dprintk("NFS: server cheating in read reply: " dprintk("NFS: server cheating in read reply: "
"count %u > recvd %u\n", count, recvd); "count %u > recvd %u\n", count, recvd);
count = recvd; count = recvd;
eof = 0; eof = 0;
} }
xdr_read_pages(xdr, count);
res->eof = eof; res->eof = eof;
res->count = count; res->count = count;
return 0; return 0;
...@@ -4952,10 +4949,6 @@ static int decode_read(struct xdr_stream *xdr, struct rpc_rqst *req, struct nfs_ ...@@ -4952,10 +4949,6 @@ static int decode_read(struct xdr_stream *xdr, struct rpc_rqst *req, struct nfs_
static int decode_readdir(struct xdr_stream *xdr, struct rpc_rqst *req, struct nfs4_readdir_res *readdir) static int decode_readdir(struct xdr_stream *xdr, struct rpc_rqst *req, struct nfs4_readdir_res *readdir)
{ {
struct xdr_buf *rcvbuf = &req->rq_rcv_buf;
struct kvec *iov = rcvbuf->head;
size_t hdrlen;
u32 recvd, pglen = rcvbuf->page_len;
int status; int status;
__be32 verf[2]; __be32 verf[2];
...@@ -4967,22 +4960,12 @@ static int decode_readdir(struct xdr_stream *xdr, struct rpc_rqst *req, struct n ...@@ -4967,22 +4960,12 @@ static int decode_readdir(struct xdr_stream *xdr, struct rpc_rqst *req, struct n
memcpy(verf, readdir->verifier.data, sizeof(verf)); memcpy(verf, readdir->verifier.data, sizeof(verf));
dprintk("%s: verifier = %08x:%08x\n", dprintk("%s: verifier = %08x:%08x\n",
__func__, verf[0], verf[1]); __func__, verf[0], verf[1]);
return xdr_read_pages(xdr, xdr->buf->page_len);
hdrlen = (char *) xdr->p - (char *) iov->iov_base;
recvd = rcvbuf->len - hdrlen;
if (pglen > recvd)
pglen = recvd;
xdr_read_pages(xdr, pglen);
return pglen;
} }
static int decode_readlink(struct xdr_stream *xdr, struct rpc_rqst *req) static int decode_readlink(struct xdr_stream *xdr, struct rpc_rqst *req)
{ {
struct xdr_buf *rcvbuf = &req->rq_rcv_buf; struct xdr_buf *rcvbuf = &req->rq_rcv_buf;
struct kvec *iov = rcvbuf->head;
size_t hdrlen;
u32 len, recvd; u32 len, recvd;
__be32 *p; __be32 *p;
int status; int status;
...@@ -5000,14 +4983,12 @@ static int decode_readlink(struct xdr_stream *xdr, struct rpc_rqst *req) ...@@ -5000,14 +4983,12 @@ static int decode_readlink(struct xdr_stream *xdr, struct rpc_rqst *req)
dprintk("nfs: server returned giant symlink!\n"); dprintk("nfs: server returned giant symlink!\n");
return -ENAMETOOLONG; return -ENAMETOOLONG;
} }
hdrlen = (char *) xdr->p - (char *) iov->iov_base; recvd = xdr_read_pages(xdr, len);
recvd = req->rq_rcv_buf.len - hdrlen;
if (recvd < len) { if (recvd < len) {
dprintk("NFS: server cheating in readlink reply: " dprintk("NFS: server cheating in readlink reply: "
"count %u > recvd %u\n", len, recvd); "count %u > recvd %u\n", len, recvd);
return -EIO; return -EIO;
} }
xdr_read_pages(xdr, len);
/* /*
* The XDR encode routine has set things up so that * The XDR encode routine has set things up so that
* the link text will be copied directly into the * the link text will be copied directly into the
...@@ -5066,7 +5047,6 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req, ...@@ -5066,7 +5047,6 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req,
__be32 *savep, *bm_p; __be32 *savep, *bm_p;
uint32_t attrlen, uint32_t attrlen,
bitmap[3] = {0}; bitmap[3] = {0};
struct kvec *iov = req->rq_rcv_buf.head;
int status; int status;
size_t page_len = xdr->buf->page_len; size_t page_len = xdr->buf->page_len;
...@@ -5089,7 +5069,6 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req, ...@@ -5089,7 +5069,6 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req,
if (unlikely(bitmap[0] & (FATTR4_WORD0_ACL - 1U))) if (unlikely(bitmap[0] & (FATTR4_WORD0_ACL - 1U)))
return -EIO; return -EIO;
if (likely(bitmap[0] & FATTR4_WORD0_ACL)) { if (likely(bitmap[0] & FATTR4_WORD0_ACL)) {
size_t hdrlen;
/* The bitmap (xdr len + bitmaps) and the attr xdr len words /* The bitmap (xdr len + bitmaps) and the attr xdr len words
* are stored with the acl data to handle the problem of * are stored with the acl data to handle the problem of
...@@ -5098,7 +5077,6 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req, ...@@ -5098,7 +5077,6 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req,
/* We ignore &savep and don't do consistency checks on /* We ignore &savep and don't do consistency checks on
* the attr length. Let userspace figure it out.... */ * the attr length. Let userspace figure it out.... */
hdrlen = (u8 *)xdr->p - (u8 *)iov->iov_base;
attrlen += res->acl_data_offset; attrlen += res->acl_data_offset;
if (attrlen > page_len) { if (attrlen > page_len) {
if (res->acl_flags & NFS4_ACL_LEN_REQUEST) { if (res->acl_flags & NFS4_ACL_LEN_REQUEST) {
...@@ -5707,9 +5685,7 @@ static int decode_layoutget(struct xdr_stream *xdr, struct rpc_rqst *req, ...@@ -5707,9 +5685,7 @@ static int decode_layoutget(struct xdr_stream *xdr, struct rpc_rqst *req,
__be32 *p; __be32 *p;
int status; int status;
u32 layout_count; u32 layout_count;
struct xdr_buf *rcvbuf = &req->rq_rcv_buf; u32 recvd;
struct kvec *iov = rcvbuf->head;
u32 hdrlen, recvd;
status = decode_op_hdr(xdr, OP_LAYOUTGET); status = decode_op_hdr(xdr, OP_LAYOUTGET);
if (status) if (status)
...@@ -5746,8 +5722,7 @@ static int decode_layoutget(struct xdr_stream *xdr, struct rpc_rqst *req, ...@@ -5746,8 +5722,7 @@ static int decode_layoutget(struct xdr_stream *xdr, struct rpc_rqst *req,
res->type, res->type,
res->layoutp->len); res->layoutp->len);
hdrlen = (u8 *) xdr->p - (u8 *) iov->iov_base; recvd = xdr_read_pages(xdr, res->layoutp->len);
recvd = req->rq_rcv_buf.len - hdrlen;
if (res->layoutp->len > recvd) { if (res->layoutp->len > recvd) {
dprintk("NFS: server cheating in layoutget reply: " dprintk("NFS: server cheating in layoutget reply: "
"layout len %u > recvd %u\n", "layout len %u > recvd %u\n",
...@@ -5755,8 +5730,6 @@ static int decode_layoutget(struct xdr_stream *xdr, struct rpc_rqst *req, ...@@ -5755,8 +5730,6 @@ static int decode_layoutget(struct xdr_stream *xdr, struct rpc_rqst *req,
return -EINVAL; return -EINVAL;
} }
xdr_read_pages(xdr, res->layoutp->len);
if (layout_count > 1) { if (layout_count > 1) {
/* We only handle a length one array at the moment. Any /* We only handle a length one array at the moment. Any
* further entries are just ignored. Note that this means * further entries are just ignored. Note that this means
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment