[PATCH 3/4] nfsd: Fix leaked memory in nfs4_make_rec_clidname

[PATCH 3/4] nfsd: Fix leaked memory in nfs4_make_rec_clidname

[Krishna Kumar]

From: Krishna Kumar <krkumar2@in.ibm.com>

cksum.data is not freed up in one error case. Compile tested.

Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---

 fs/nfsd/nfs4recover.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -ruNp linux-2.6.27.org/fs/nfsd/nfs4recover.c linux-2.6.27.new/fs/nfsd/nfs4recover.c
--- linux-2.6.27.org/fs/nfsd/nfs4recover.c	2008-10-20 10:47:15.000000000 +0530
+++ linux-2.6.27.new/fs/nfsd/nfs4recover.c	2008-10-20 10:48:38.000000000 +0530
@@ -110,9 +110,9 @@ nfs4_make_rec_clidname(char *dname, stru
 
 	md5_to_hex(dname, cksum.data);
 
-	kfree(cksum.data);
 	status = nfs_ok;
 out:
+	kfree(cksum.data);
 	crypto_free_hash(desc.tfm);
 out_no_tfm:
 	return status;

[PATCH 4/4] nfsd: Minor cleanup of _get_posix_acl

[Krishna Kumar]

From: Krishna Kumar <krkumar2@in.ibm.com>

Rewrite error case to not check twice for !buflen. Compile tested.

Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
 fs/nfsd/vfs.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff -ruNp linux-2.6.27.org/fs/nfsd/vfs.c linux-2.6.27.new/fs/nfsd/vfs.c
--- linux-2.6.27.org/fs/nfsd/vfs.c	2008-10-20 10:47:19.000000000 +0530
+++ linux-2.6.27.new/fs/nfsd/vfs.c	2008-10-20 10:48:32.000000000 +0530
@@ -503,10 +503,11 @@ _get_posix_acl(struct dentry *dentry, ch
 	int buflen;
 
 	buflen = nfsd_getxattr(dentry, key, &buf);
-	if (!buflen)
-		buflen = -ENODATA;
-	if (buflen <= 0)
+	if (buflen <= 0) {
+		if (!buflen)
+			buflen = -ENODATA;
 		return ERR_PTR(buflen);
+	}
 
 	pacl = posix_acl_from_xattr(buf, buflen);
 	kfree(buf);

Re: [PATCH 4/4] nfsd: Minor cleanup of _get_posix_acl

[J. Bruce Fields]

On Mon, Oct 20, 2008 at 11:47:21AM +0530, Krishna Kumar wrote:
> From: Krishna Kumar <krkumar2@in.ibm.com>
> 
> Rewrite error case to not check twice for !buflen. Compile tested.

Is the current code really a problem?

Possibly more interesting to look into:

	- Can nfsd_getxattr actually return NULL?
	- Why do we need to return ENODATA in that case?
	  (nfsd_get_posix_acl, for example, seems to just end up
	  returning -EINVAL in that case.)

--b.

> 
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> ---
>  fs/nfsd/vfs.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff -ruNp linux-2.6.27.org/fs/nfsd/vfs.c linux-2.6.27.new/fs/nfsd/vfs.c
> --- linux-2.6.27.org/fs/nfsd/vfs.c	2008-10-20 10:47:19.000000000 +0530
> +++ linux-2.6.27.new/fs/nfsd/vfs.c	2008-10-20 10:48:32.000000000 +0530
> @@ -503,10 +503,11 @@ _get_posix_acl(struct dentry *dentry, ch
>  	int buflen;
>  
>  	buflen = nfsd_getxattr(dentry, key, &buf);
> -	if (!buflen)
> -		buflen = -ENODATA;
> -	if (buflen <= 0)
> +	if (buflen <= 0) {
> +		if (!buflen)
> +			buflen = -ENODATA;
>  		return ERR_PTR(buflen);
> +	}
>  
>  	pacl = posix_acl_from_xattr(buf, buflen);
>  	kfree(buf);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] nfsd: Fix leaked memory in nfs4_make_rec_clidname

[J. Bruce Fields]

On Mon, Oct 20, 2008 at 11:47:09AM +0530, Krishna Kumar wrote:
> From: Krishna Kumar <krkumar2@in.ibm.com>
> 
> cksum.data is not freed up in one error case. Compile tested.
> 

Thanks, applied.

--b.

> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> ---
> 
>  fs/nfsd/nfs4recover.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff -ruNp linux-2.6.27.org/fs/nfsd/nfs4recover.c linux-2.6.27.new/fs/nfsd/nfs4recover.c
> --- linux-2.6.27.org/fs/nfsd/nfs4recover.c	2008-10-20 10:47:15.000000000 +0530
> +++ linux-2.6.27.new/fs/nfsd/nfs4recover.c	2008-10-20 10:48:38.000000000 +0530
> @@ -110,9 +110,9 @@ nfs4_make_rec_clidname(char *dname, stru
>  
>  	md5_to_hex(dname, cksum.data);
>  
> -	kfree(cksum.data);
>  	status = nfs_ok;
>  out:
> +	kfree(cksum.data);
>  	crypto_free_hash(desc.tfm);
>  out_no_tfm:
>  	return status;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html