NFSv3: Fix posix ACL code
authorTrond Myklebust <Trond.Myklebust@netapp.com>
Wed, 11 Mar 2009 00:33:18 +0000 (20:33 -0400)
committerTrond Myklebust <Trond.Myklebust@netapp.com>
Wed, 11 Mar 2009 00:33:18 +0000 (20:33 -0400)
Fix a memory leak due to allocation in the XDR layer. In cases where the
RPC call needs to be retransmitted, we end up allocating new pages without
clearing the old ones. Fix this by moving the allocation into
nfs3_proc_setacls().

Also fix an issue discovered by Kevin Rudd, whereby the amount of memory
reserved for the acls in the xdr_buf->head was miscalculated, and causing
corruption.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
fs/nfs/nfs3acl.c
fs/nfs/nfs3xdr.c
include/linux/nfs_xdr.h
include/linux/nfsacl.h

index cef62557c87daeadab3f33fa7bd468b979ae59f6..6bbf0e6daad27a3ad91c6b66378ce70012b66304 100644 (file)
@@ -292,7 +292,7 @@ static int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
 {
        struct nfs_server *server = NFS_SERVER(inode);
        struct nfs_fattr fattr;
-       struct page *pages[NFSACL_MAXPAGES] = { };
+       struct page *pages[NFSACL_MAXPAGES];
        struct nfs3_setaclargs args = {
                .inode = inode,
                .mask = NFS_ACL,
@@ -303,7 +303,7 @@ static int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
                .rpc_argp       = &args,
                .rpc_resp       = &fattr,
        };
-       int status, count;
+       int status;
 
        status = -EOPNOTSUPP;
        if (!nfs_server_capable(inode, NFS_CAP_ACLS))
@@ -319,6 +319,20 @@ static int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
        if (S_ISDIR(inode->i_mode)) {
                args.mask |= NFS_DFACL;
                args.acl_default = dfacl;
+               args.len = nfsacl_size(acl, dfacl);
+       } else
+               args.len = nfsacl_size(acl, NULL);
+
+       if (args.len > NFS_ACL_INLINE_BUFSIZE) {
+               unsigned int npages = 1 + ((args.len - 1) >> PAGE_SHIFT);
+
+               status = -ENOMEM;
+               do {
+                       args.pages[args.npages] = alloc_page(GFP_KERNEL);
+                       if (args.pages[args.npages] == NULL)
+                               goto out_freepages;
+                       args.npages++;
+               } while (args.npages < npages);
        }
 
        dprintk("NFS call setacl\n");
@@ -329,10 +343,6 @@ static int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
        nfs_zap_acl_cache(inode);
        dprintk("NFS reply setacl: %d\n", status);
 
-       /* pages may have been allocated at the xdr layer. */
-       for (count = 0; count < NFSACL_MAXPAGES && args.pages[count]; count++)
-               __free_page(args.pages[count]);
-
        switch (status) {
                case 0:
                        status = nfs_refresh_inode(inode, &fattr);
@@ -346,6 +356,11 @@ static int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
                case -ENOTSUPP:
                        status = -EOPNOTSUPP;
        }
+out_freepages:
+       while (args.npages != 0) {
+               args.npages--;
+               __free_page(args.pages[args.npages]);
+       }
 out:
        return status;
 }
index 11cdddec1432cfae1199aa2f89257ce00dd033d1..6cdeacffde469143045588c6b20e6cff72cc586a 100644 (file)
 #define NFS3_commitres_sz      (1+NFS3_wcc_data_sz+2)
 
 #define ACL3_getaclargs_sz     (NFS3_fh_sz+1)
-#define ACL3_setaclargs_sz     (NFS3_fh_sz+1+2*(2+5*3))
-#define ACL3_getaclres_sz      (1+NFS3_post_op_attr_sz+1+2*(2+5*3))
+#define ACL3_setaclargs_sz     (NFS3_fh_sz+1+ \
+                               XDR_QUADLEN(NFS_ACL_INLINE_BUFSIZE))
+#define ACL3_getaclres_sz      (1+NFS3_post_op_attr_sz+1+ \
+                               XDR_QUADLEN(NFS_ACL_INLINE_BUFSIZE))
 #define ACL3_setaclres_sz      (1+NFS3_post_op_attr_sz)
 
 /*
@@ -703,28 +705,18 @@ nfs3_xdr_setaclargs(struct rpc_rqst *req, __be32 *p,
                    struct nfs3_setaclargs *args)
 {
        struct xdr_buf *buf = &req->rq_snd_buf;
-       unsigned int base, len_in_head, len = nfsacl_size(
-               (args->mask & NFS_ACL)   ? args->acl_access  : NULL,
-               (args->mask & NFS_DFACL) ? args->acl_default : NULL);
-       int count, err;
+       unsigned int base;
+       int err;
 
        p = xdr_encode_fhandle(p, NFS_FH(args->inode));
        *p++ = htonl(args->mask);
-       base = (char *)p - (char *)buf->head->iov_base;
-       /* put as much of the acls into head as possible. */
-       len_in_head = min_t(unsigned int, buf->head->iov_len - base, len);
-       len -= len_in_head;
-       req->rq_slen = xdr_adjust_iovec(req->rq_svec, p + (len_in_head >> 2));
-
-       for (count = 0; (count << PAGE_SHIFT) < len; count++) {
-               args->pages[count] = alloc_page(GFP_KERNEL);
-               if (!args->pages[count]) {
-                       while (count)
-                               __free_page(args->pages[--count]);
-                       return -ENOMEM;
-               }
-       }
-       xdr_encode_pages(buf, args->pages, 0, len);
+       req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
+       base = req->rq_slen;
+
+       if (args->npages != 0)
+               xdr_encode_pages(buf, args->pages, 0, args->len);
+       else
+               req->rq_slen += args->len;
 
        err = nfsacl_encode(buf, base, args->inode,
                            (args->mask & NFS_ACL) ?
index a550b528319f9db55eff12b3d84dcee03da4cc0e..2e5f00066afd2a5e625b6bfc34de395a65929cc8 100644 (file)
@@ -406,6 +406,8 @@ struct nfs3_setaclargs {
        int                     mask;
        struct posix_acl *      acl_access;
        struct posix_acl *      acl_default;
+       size_t                  len;
+       unsigned int            npages;
        struct page **          pages;
 };
 
index 54487a99beb8b61d5bf2351f63ef9549fc094a89..43011b69297c39adb95e9afbfae4990e17620fe3 100644 (file)
@@ -37,6 +37,9 @@
 #define NFSACL_MAXPAGES                ((2*(8+12*NFS_ACL_MAX_ENTRIES) + PAGE_SIZE-1) \
                                 >> PAGE_SHIFT)
 
+#define NFS_ACL_MAX_ENTRIES_INLINE     (5)
+#define NFS_ACL_INLINE_BUFSIZE ((2*(2+3*NFS_ACL_MAX_ENTRIES_INLINE)) << 2)
+
 static inline unsigned int
 nfsacl_size(struct posix_acl *acl_access, struct posix_acl *acl_default)
 {