Commit e8896495 authored by David Howells's avatar David Howells Committed by Trond Myklebust

NFS: Check lengths more thoroughly in NFS4 readdir XDR decode

Check the bounds of length specifiers more thoroughly in the XDR decoding of
NFS4 readdir reply data.

Currently, if the server returns a bitmap or attr length that causes the
current decode point pointer to wrap, this could go undetected (consider a
small "negative" length on a 32-bit machine).

Also add a check into the main XDR decode handler to make sure that the amount
of data is a multiple of four bytes (as specified by RFC-1014).  This makes
sure that we can do u32* pointer subtraction in the NFS client without risking
an undefined result (the result is undefined if the pointers are not correctly
aligned with respect to one another).
Signed-Off-By: default avatarDavid Howells <dhowells@redhat.com>
Signed-off-by: default avatarTrond Myklebust <Trond.Myklebust@netapp.com>
(cherry picked from 5861fddd64a7eaf7e8b1a9997455a24e7f688092 commit)
parent 3cedf13a
...@@ -3355,7 +3355,7 @@ static int decode_readdir(struct xdr_stream *xdr, struct rpc_rqst *req, struct n ...@@ -3355,7 +3355,7 @@ static int decode_readdir(struct xdr_stream *xdr, struct rpc_rqst *req, struct n
struct kvec *iov = rcvbuf->head; struct kvec *iov = rcvbuf->head;
unsigned int nr, pglen = rcvbuf->page_len; unsigned int nr, pglen = rcvbuf->page_len;
uint32_t *end, *entry, *p, *kaddr; uint32_t *end, *entry, *p, *kaddr;
uint32_t len, attrlen; uint32_t len, attrlen, xlen;
int hdrlen, recvd, status; int hdrlen, recvd, status;
status = decode_op_hdr(xdr, OP_READDIR); status = decode_op_hdr(xdr, OP_READDIR);
...@@ -3377,10 +3377,10 @@ static int decode_readdir(struct xdr_stream *xdr, struct rpc_rqst *req, struct n ...@@ -3377,10 +3377,10 @@ static int decode_readdir(struct xdr_stream *xdr, struct rpc_rqst *req, struct n
BUG_ON(pglen + readdir->pgbase > PAGE_CACHE_SIZE); BUG_ON(pglen + readdir->pgbase > PAGE_CACHE_SIZE);
kaddr = p = (uint32_t *) kmap_atomic(page, KM_USER0); kaddr = p = (uint32_t *) kmap_atomic(page, KM_USER0);
end = (uint32_t *) ((char *)p + pglen + readdir->pgbase); end = p + ((pglen + readdir->pgbase) >> 2);
entry = p; entry = p;
for (nr = 0; *p++; nr++) { for (nr = 0; *p++; nr++) {
if (p + 3 > end) if (end - p < 3)
goto short_pkt; goto short_pkt;
dprintk("cookie = %Lu, ", *((unsigned long long *)p)); dprintk("cookie = %Lu, ", *((unsigned long long *)p));
p += 2; /* cookie */ p += 2; /* cookie */
...@@ -3389,18 +3389,19 @@ static int decode_readdir(struct xdr_stream *xdr, struct rpc_rqst *req, struct n ...@@ -3389,18 +3389,19 @@ static int decode_readdir(struct xdr_stream *xdr, struct rpc_rqst *req, struct n
printk(KERN_WARNING "NFS: giant filename in readdir (len 0x%x)\n", len); printk(KERN_WARNING "NFS: giant filename in readdir (len 0x%x)\n", len);
goto err_unmap; goto err_unmap;
} }
dprintk("filename = %*s\n", len, (char *)p); xlen = XDR_QUADLEN(len);
p += XDR_QUADLEN(len); if (end - p < xlen + 1)
if (p + 1 > end)
goto short_pkt; goto short_pkt;
dprintk("filename = %*s\n", len, (char *)p);
p += xlen;
len = ntohl(*p++); /* bitmap length */ len = ntohl(*p++); /* bitmap length */
p += len; if (end - p < len + 1)
if (p + 1 > end)
goto short_pkt; goto short_pkt;
p += len;
attrlen = XDR_QUADLEN(ntohl(*p++)); attrlen = XDR_QUADLEN(ntohl(*p++));
p += attrlen; /* attributes */ if (end - p < attrlen + 2)
if (p + 2 > end)
goto short_pkt; goto short_pkt;
p += attrlen; /* attributes */
entry = p; entry = p;
} }
if (!nr && (entry[0] != 0 || entry[1] == 0)) if (!nr && (entry[0] != 0 || entry[1] == 0))
......
...@@ -1181,6 +1181,17 @@ call_verify(struct rpc_task *task) ...@@ -1181,6 +1181,17 @@ call_verify(struct rpc_task *task)
u32 *p = iov->iov_base, n; u32 *p = iov->iov_base, n;
int error = -EACCES; int error = -EACCES;
if ((task->tk_rqstp->rq_rcv_buf.len & 3) != 0) {
/* RFC-1014 says that the representation of XDR data must be a
* multiple of four bytes
* - if it isn't pointer subtraction in the NFS client may give
* undefined results
*/
printk(KERN_WARNING
"call_verify: XDR representation not a multiple of"
" 4 bytes: 0x%x\n", task->tk_rqstp->rq_rcv_buf.len);
goto out_eio;
}
if ((len -= 3) < 0) if ((len -= 3) < 0)
goto out_overflow; goto out_overflow;
p += 1; /* skip XID */ p += 1; /* skip XID */
......
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