Skip to contentSkip to conversation

nfsd: Fix leaked memory in nfs4_make_rec_clidname

PATCH Applied Krishna Kumar 2 participants 4 messages Oct 20, 2008 2 messages not in the archive

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

Krishna Kumar

author
#0 lore raw

PATCH 3/4 nfsd: Fix leaked memory in nfs4_make_rec_clidname

From: Krishna Kumar <[email protected]>

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

Signed-off-by Krishna Kumar <[email protected]>

---

 fs/nfsd/nfs4recover.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/nfsd/nfs4recover.c +1 -1
diff -ruNp linux-2.6.27.org/fs/nfsd/nfs4recover.c linux-2.6.27.new/fs/nfsd/nfs4recover.c
@@ -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;

A message that is not in the archive (sent off-list or not yet received): 20081020061709.18370.85373.sendpatchset-bi+AKbBUZKY6gyzm1THtWbp2dZbC/[email protected]

author
#2 lore raw

PATCH 4/4 nfsd: Minor cleanup of _get_posix_acl

From: Krishna Kumar <[email protected]>

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

Signed-off-by Krishna Kumar <[email protected]>
---
 fs/nfsd/vfs.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
fs/nfsd/vfs.c +4 -3
diff -ruNp linux-2.6.27.org/fs/nfsd/vfs.c linux-2.6.27.new/fs/nfsd/vfs.c
@@ -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);

A message that is not in the archive (sent off-list or not yet received): 20081020061721.18370.93671.sendpatchset-bi+AKbBUZKY6gyzm1THtWbp2dZbC/[email protected]

#4 lore raw

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

Quoted 3 lines · Krishna Kumar

From: Krishna Kumar <[email protected]>

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.

Quoted 28 lines
Signed-off-by Krishna Kumar <[email protected]>
---
 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

fs/nfsd/vfs.c +4 -4
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Applied
#5 lore raw
In reply to a message not in the archive
Quoted 4 lines · Krishna Kumar

From: Krishna Kumar <[email protected]>

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

Thanks, applied.

--b.

Quoted 24 lines
Signed-off-by Krishna Kumar <[email protected]>

---

 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

fs/nfsd/nfs4recover.c +1 -2
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html