linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sachin Prabhu <sprabhu@redhat.com>
To: Linux NFS mailing list <linux-nfs@vger.kernel.org>
Cc: Trond Myklebust <trond.myklebust@netapp.com>
Subject: [PATCH] Fix array overflow in __nfs4_get_acl_uncached
Date: Tue, 24 Jul 2012 12:36:25 +0100	[thread overview]
Message-ID: <1343129785-4538-1-git-send-email-sprabhu@redhat.com> (raw)

This fixes a bug introduced by commit
5a00689930ab975fdd1b37b034475017e460cf2a

Bruce Fields pointed out that the changes introduced by the patch will
cause the array npages to overflow if a size requested is >=
XATTR_SIZE_MAX.

The patch also fixes the check for the length of the ACL returned. The
flag ACL_LEN_REQUEST has been renamed to NFS4_ACL_LEN_ONLY and apart
from indicating that the user is just interested in the ACL length when
making a request, it is also used to indicate if the xdr pages buffer
returned in response holds the complete ACL or just the length.

Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
---
 fs/nfs/nfs4proc.c       |   18 +++++++++---------
 fs/nfs/nfs4xdr.c        |    4 +++-
 include/linux/nfs_xdr.h |    2 +-
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 15fc7e4..e19c322 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3726,16 +3726,16 @@ out:
 /*
  * The getxattr API returns the required buffer length when called with a
  * NULL buf. The NFSv4 acl tool then calls getxattr again after allocating
- * the required buf.  On a NULL buf, we send a page of data to the server
- * guessing that the ACL request can be serviced by a page. If so, we cache
- * up to the page of ACL data, and the 2nd call to getxattr is serviced by
- * the cache. If not so, we throw away the page, and cache the required
- * length. The next getxattr call will then produce another round trip to
- * the server, this time with the input buf of the required size.
+ * the required buf.  On a NULL buf, we allocate 2 pages guessing that the
+ * ACL request can be serviced by those pages. If so, we cache the ACL data,
+ * and the 2nd call to getxattr is serviced by the cache. If not so, we throw
+ * away the pages, and cache the required length. The next getxattr call will
+ * then produce another round trip to the server, this time with the input buf
+ * of the required size.
  */
 static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t buflen)
 {
-	struct page *pages[NFS4ACL_MAXPAGES] = {NULL, };
+	struct page *pages[NFS4ACL_MAXPAGES+1] = {NULL, };
 	struct nfs_getaclargs args = {
 		.fh = NFS_FH(inode),
 		.acl_pages = pages,
@@ -3777,7 +3777,7 @@ static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t bu
 	/* Let decode_getfacl know not to fail if the ACL data is larger than
 	 * the page we send as a guess */
 	if (buf == NULL)
-		res.acl_flags |= NFS4_ACL_LEN_REQUEST;
+		res.acl_flags |= NFS4_ACL_LEN_ONLY;
 
 	dprintk("%s  buf %p buflen %zu npages %d args.acl_len %zu\n",
 		__func__, buf, buflen, npages, args.acl_len);
@@ -3787,7 +3787,7 @@ static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t bu
 		goto out_free;
 
 	acl_len = res.acl_len - res.acl_data_offset;
-	if (acl_len > args.acl_len)
+	if (res.acl_flags & NFS4_ACL_LEN_ONLY)
 		nfs4_write_cached_acl(inode, NULL, 0, acl_len);
 	else
 		nfs4_write_cached_acl(inode, pages, res.acl_data_offset,
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 18fae29..a88fcd0 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -5101,7 +5101,7 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req,
 		hdrlen = (u8 *)xdr->p - (u8 *)iov->iov_base;
 		attrlen += res->acl_data_offset;
 		if (attrlen > page_len) {
-			if (res->acl_flags & NFS4_ACL_LEN_REQUEST) {
+			if (res->acl_flags & NFS4_ACL_LEN_ONLY) {
 				/* getxattr interface called with a NULL buf */
 				res->acl_len = attrlen;
 				goto out;
@@ -5110,6 +5110,8 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req,
 					attrlen, page_len);
 			return -EINVAL;
 		}
+		/* At this stage we have the complete ACL */
+		res->acl_flags &= ~NFS4_ACL_LEN_ONLY;
 		xdr_read_pages(xdr, attrlen);
 		res->acl_len = attrlen;
 	} else
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 8aadd90..39c8483 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -648,7 +648,7 @@ struct nfs_getaclargs {
 };
 
 /* getxattr ACL interface flags */
-#define NFS4_ACL_LEN_REQUEST	0x0001	/* zero length getxattr buffer */
+#define NFS4_ACL_LEN_ONLY	0x0001	/* zero length getxattr buffer */
 struct nfs_getaclres {
 	size_t				acl_len;
 	size_t				acl_data_offset;
-- 
1.7.10.4


             reply	other threads:[~2012-07-24 11:36 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-24 11:36 Sachin Prabhu [this message]
2012-07-24 17:22 ` [PATCH] Fix array overflow in __nfs4_get_acl_uncached J. Bruce Fields
2012-07-30 21:54 ` Myklebust, Trond
2012-07-31 21:04   ` Sachin Prabhu
2012-07-31 21:47     ` Myklebust, Trond

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1343129785-4538-1-git-send-email-sprabhu@redhat.com \
    --to=sprabhu@redhat.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@netapp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).