NFSv3: Fix posix ACL code

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>

NFSv3: Fix posix ACL code
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>
Trond Myklebust
1 parent ef95d31e6d
Showing 4 changed files with 39 additions and 27 deletions Side-by-side Diff
fs/nfs/nfs3acl.c
fs/nfs/nfs3xdr.c
include/linux/nfs_xdr.h
include/linux/nfsacl.h
@@ -292,7 +292,7 @@
 {
 	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 @@
 		.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 @@
 	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 @@
 	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);
@@ -345,6 +355,11 @@
 			server->caps &= ~NFS_CAP_ACLS;
 		case -ENOTSUPP:
 			status = -EOPNOTSUPP;
+	}
+out_freepages:
+	while (args.npages != 0) {
+		args.npages--;
+		__free_page(args.pages[args.npages]);
 	}
 out:
 	return status;
@@ -82,8 +82,10 @@
 #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 @@
                    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));
+	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
+	base = req->rq_slen;
  
-	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);
+	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) ?
@@ -406,6 +406,8 @@
 	int			mask;
 	struct posix_acl *	acl_access;
 	struct posix_acl *	acl_default;
+	size_t			len;
+	unsigned int		npages;
 	struct page **		pages;
 };
  
@@ -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)
 {
...	...	@@ -292,7 +292,7 @@
292	292	{
293	293	struct nfs_server *server = NFS_SERVER(inode);
294	294	struct nfs_fattr fattr;
295		- struct page *pages[NFSACL_MAXPAGES] = { };
	295	+ struct page *pages[NFSACL_MAXPAGES];
296	296	struct nfs3_setaclargs args = {
297	297	.inode = inode,
298	298	.mask = NFS_ACL,
...	...	@@ -303,7 +303,7 @@
303	303	.rpc_argp = &args,
304	304	.rpc_resp = &fattr,
305	305	};
306		- int status, count;
	306	+ int status;
307	307
308	308	status = -EOPNOTSUPP;
309	309	if (!nfs_server_capable(inode, NFS_CAP_ACLS))
...	...	@@ -319,6 +319,20 @@
319	319	if (S_ISDIR(inode->i_mode)) {
320	320	args.mask \|= NFS_DFACL;
321	321	args.acl_default = dfacl;
	322	+ args.len = nfsacl_size(acl, dfacl);
	323	+ } else
	324	+ args.len = nfsacl_size(acl, NULL);
	325	+
	326	+ if (args.len > NFS_ACL_INLINE_BUFSIZE) {
	327	+ unsigned int npages = 1 + ((args.len - 1) >> PAGE_SHIFT);
	328	+
	329	+ status = -ENOMEM;
	330	+ do {
	331	+ args.pages[args.npages] = alloc_page(GFP_KERNEL);
	332	+ if (args.pages[args.npages] == NULL)
	333	+ goto out_freepages;
	334	+ args.npages++;
	335	+ } while (args.npages < npages);
322	336	}
323	337
324	338	dprintk("NFS call setacl\n");
...	...	@@ -329,10 +343,6 @@
329	343	nfs_zap_acl_cache(inode);
330	344	dprintk("NFS reply setacl: %d\n", status);
331	345
332		- /* pages may have been allocated at the xdr layer. */
333		- for (count = 0; count < NFSACL_MAXPAGES && args.pages[count]; count++)
334		- __free_page(args.pages[count]);
335		-
336	346	switch (status) {
337	347	case 0:
338	348	status = nfs_refresh_inode(inode, &fattr);
...	...	@@ -345,6 +355,11 @@
345	355	server->caps &= ~NFS_CAP_ACLS;
346	356	case -ENOTSUPP:
347	357	status = -EOPNOTSUPP;
	358	+ }
	359	+out_freepages:
	360	+ while (args.npages != 0) {
	361	+ args.npages--;
	362	+ __free_page(args.pages[args.npages]);
348	363	}
349	364	out:
350	365	return status;
...	...	@@ -82,8 +82,10 @@
82	82	#define NFS3_commitres_sz (1+NFS3_wcc_data_sz+2)
83	83
84	84	#define ACL3_getaclargs_sz (NFS3_fh_sz+1)
85		-#define ACL3_setaclargs_sz (NFS3_fh_sz+1+2(2+53))
86		-#define ACL3_getaclres_sz (1+NFS3_post_op_attr_sz+1+2(2+53))
	85	+#define ACL3_setaclargs_sz (NFS3_fh_sz+1+ \
	86	+ XDR_QUADLEN(NFS_ACL_INLINE_BUFSIZE))
	87	+#define ACL3_getaclres_sz (1+NFS3_post_op_attr_sz+1+ \
	88	+ XDR_QUADLEN(NFS_ACL_INLINE_BUFSIZE))
87	89	#define ACL3_setaclres_sz (1+NFS3_post_op_attr_sz)
88	90
89	91	/*
90	92
91	93
...	...	@@ -703,28 +705,18 @@
703	705	struct nfs3_setaclargs *args)
704	706	{
705	707	struct xdr_buf *buf = &req->rq_snd_buf;
706		- unsigned int base, len_in_head, len = nfsacl_size(
707		- (args->mask & NFS_ACL) ? args->acl_access : NULL,
708		- (args->mask & NFS_DFACL) ? args->acl_default : NULL);
709		- int count, err;
	708	+ unsigned int base;
	709	+ int err;
710	710
711	711	p = xdr_encode_fhandle(p, NFS_FH(args->inode));
712	712	*p++ = htonl(args->mask);
713		- base = (char )p - (char )buf->head->iov_base;
714		- /* put as much of the acls into head as possible. */
715		- len_in_head = min_t(unsigned int, buf->head->iov_len - base, len);
716		- len -= len_in_head;
717		- req->rq_slen = xdr_adjust_iovec(req->rq_svec, p + (len_in_head >> 2));
	713	+ req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
	714	+ base = req->rq_slen;
718	715
719		- for (count = 0; (count << PAGE_SHIFT) < len; count++) {
720		- args->pages[count] = alloc_page(GFP_KERNEL);
721		- if (!args->pages[count]) {
722		- while (count)
723		- __free_page(args->pages[--count]);
724		- return -ENOMEM;
725		- }
726		- }
727		- xdr_encode_pages(buf, args->pages, 0, len);
	716	+ if (args->npages != 0)
	717	+ xdr_encode_pages(buf, args->pages, 0, args->len);
	718	+ else
	719	+ req->rq_slen += args->len;
728	720
729	721	err = nfsacl_encode(buf, base, args->inode,
730	722	(args->mask & NFS_ACL) ?
...	...	@@ -406,6 +406,8 @@
406	406	int mask;
407	407	struct posix_acl * acl_access;
408	408	struct posix_acl * acl_default;
	409	+ size_t len;
	410	+ unsigned int npages;
409	411	struct page ** pages;
410	412	};
411	413
...	...	@@ -37,6 +37,9 @@
37	37	#define NFSACL_MAXPAGES ((2(8+12NFS_ACL_MAX_ENTRIES) + PAGE_SIZE-1) \
38	38	>> PAGE_SHIFT)
39	39
	40	+#define NFS_ACL_MAX_ENTRIES_INLINE (5)
	41	+#define NFS_ACL_INLINE_BUFSIZE ((2(2+3NFS_ACL_MAX_ENTRIES_INLINE)) << 2)
	42	+
40	43	static inline unsigned int
41	44	nfsacl_size(struct posix_acl acl_access, struct posix_acl acl_default)
42	45	{