Commit 5a006899 authored by Sachin Prabhu's avatar Sachin Prabhu Committed by Trond Myklebust

Avoid reading past buffer when calling GETACL

Bug noticed in commit
bf118a34

When calling GETACL, if the size of the bitmap array, the length
attribute and the acl returned by the server is greater than the
allocated buffer(args.acl_len), we can Oops with a General Protection
fault at _copy_from_pages() when we attempt to read past the pages
allocated.

This patch allocates an extra PAGE for the bitmap and checks to see that
the bitmap + attribute_length + ACLs don't exceed the buffer space
allocated to it.
Signed-off-by: default avatarSachin Prabhu <sprabhu@redhat.com>
Reported-by: default avatarJian Li <jiali@redhat.com>
[Trond: Fixed a size_t vs unsigned int printk() warning]
Signed-off-by: default avatarTrond Myklebust <Trond.Myklebust@netapp.com>
parent 10bd295a
...@@ -3684,19 +3684,23 @@ static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t bu ...@@ -3684,19 +3684,23 @@ static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t bu
if (npages == 0) if (npages == 0)
npages = 1; npages = 1;
/* Add an extra page to handle the bitmap returned */
npages++;
for (i = 0; i < npages; i++) { for (i = 0; i < npages; i++) {
pages[i] = alloc_page(GFP_KERNEL); pages[i] = alloc_page(GFP_KERNEL);
if (!pages[i]) if (!pages[i])
goto out_free; goto out_free;
} }
if (npages > 1) {
/* for decoding across pages */ /* for decoding across pages */
res.acl_scratch = alloc_page(GFP_KERNEL); res.acl_scratch = alloc_page(GFP_KERNEL);
if (!res.acl_scratch) if (!res.acl_scratch)
goto out_free; goto out_free;
}
args.acl_len = npages * PAGE_SIZE; args.acl_len = npages * PAGE_SIZE;
args.acl_pgbase = 0; args.acl_pgbase = 0;
/* Let decode_getfacl know not to fail if the ACL data is larger than /* Let decode_getfacl know not to fail if the ACL data is larger than
* the page we send as a guess */ * the page we send as a guess */
if (buf == NULL) if (buf == NULL)
......
...@@ -4902,11 +4902,19 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req, ...@@ -4902,11 +4902,19 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req,
bitmap[3] = {0}; bitmap[3] = {0};
struct kvec *iov = req->rq_rcv_buf.head; struct kvec *iov = req->rq_rcv_buf.head;
int status; int status;
size_t page_len = xdr->buf->page_len;
res->acl_len = 0; res->acl_len = 0;
if ((status = decode_op_hdr(xdr, OP_GETATTR)) != 0) if ((status = decode_op_hdr(xdr, OP_GETATTR)) != 0)
goto out; goto out;
bm_p = xdr->p; bm_p = xdr->p;
res->acl_data_offset = be32_to_cpup(bm_p) + 2;
res->acl_data_offset <<= 2;
/* Check if the acl data starts beyond the allocated buffer */
if (res->acl_data_offset > page_len)
return -ERANGE;
if ((status = decode_attr_bitmap(xdr, bitmap)) != 0) if ((status = decode_attr_bitmap(xdr, bitmap)) != 0)
goto out; goto out;
if ((status = decode_attr_length(xdr, &attrlen, &savep)) != 0) if ((status = decode_attr_length(xdr, &attrlen, &savep)) != 0)
...@@ -4916,28 +4924,24 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req, ...@@ -4916,28 +4924,24 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req,
return -EIO; return -EIO;
if (likely(bitmap[0] & FATTR4_WORD0_ACL)) { if (likely(bitmap[0] & FATTR4_WORD0_ACL)) {
size_t hdrlen; size_t hdrlen;
u32 recvd;
/* 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
* variable length bitmaps.*/ * variable length bitmaps.*/
xdr->p = bm_p; xdr->p = bm_p;
res->acl_data_offset = be32_to_cpup(bm_p) + 2;
res->acl_data_offset <<= 2;
/* 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; hdrlen = (u8 *)xdr->p - (u8 *)iov->iov_base;
attrlen += res->acl_data_offset; attrlen += res->acl_data_offset;
recvd = req->rq_rcv_buf.len - hdrlen; if (attrlen > page_len) {
if (attrlen > recvd) {
if (res->acl_flags & NFS4_ACL_LEN_REQUEST) { if (res->acl_flags & NFS4_ACL_LEN_REQUEST) {
/* getxattr interface called with a NULL buf */ /* getxattr interface called with a NULL buf */
res->acl_len = attrlen; res->acl_len = attrlen;
goto out; goto out;
} }
dprintk("NFS: acl reply: attrlen %u > recvd %u\n", dprintk("NFS: acl reply: attrlen %zu > page_len %u\n",
attrlen, recvd); attrlen, page_len);
return -EINVAL; return -EINVAL;
} }
xdr_read_pages(xdr, attrlen); xdr_read_pages(xdr, attrlen);
......
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