nfs: check kmalloc() return

PATCH In review Dan Carpenter 4 participants 6 messages Oct 28, 2010 – Oct 29, 2010

Subsystems (3)

Kernel janitors NFS client NFS server

lore

Share & tools

lore

Dan Carpenter

author

2010-10-28 04:44 UTC #0 lore raw

PATCH 1/1 nfs: check kmalloc() return

The decode_and_add_ds() should return NULL on failure.

Signed-off-by Dan Carpenter <[email protected]>

fs/nfs/nfs4filelayoutdev.c +2 -0

index 51fe64a..098113c 100644
@@ -219,6 +219,8 @@ decode_and_add_ds(__be32 **pp, struct inode *inode)
 		goto out_err;
 	}
 	buf = kmalloc(rlen + 1, GFP_KERNEL);
+	if (!buf)
+		goto out_err;
 	buf[rlen] = '\0';
 	memcpy(buf, r_addr, rlen);

walter harms

2010-10-28 07:16 UTC #1 lore raw

Quoted 9 lines · Dan Carpenter

The decode_and_add_ds() should return NULL on failure.
Signed-off-by Dan Carpenter <[email protected]>
fs/nfs/nfs4filelayoutdev.c +0 -0
index 51fe64a..098113c 100644

fs/nfs/nfs4filelayoutdev.c decode_and_add_ds(__be32 **pp, struct inode *inode) · line 226

Quoted 8 lines

fs/nfs/nfs4filelayoutdev.c +2 -0

@@ -219,6 +219,8 @@ decode_and_add_ds(__be32 **pp, struct inode *inode)
 		goto out_err;
 	}
 	buf = kmalloc(rlen + 1, GFP_KERNEL);
+	if (!buf)
+		goto out_err;
 	buf[rlen] = '\0';
 	memcpy(buf, r_addr, rlen);

it seems that r_addr is a string, then kstdup() is emulated here.

re,
wh

Quoted 6 lines

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Trond Myklebust

2010-10-28 13:41 UTC #2 lore raw

Quoted 22 lines · walter harms

Quoted 9 lines · Dan Carpenter
The decode_and_add_ds() should return NULL on failure.
Signed-off-by Dan Carpenter <[email protected]>
fs/nfs/nfs4filelayoutdev.c +0 -0
index 51fe64a..098113c 100644
fs/nfs/nfs4filelayoutdev.c decode_and_add_ds(__be32 **pp, struct inode *inode) · line 226
Quoted 8 lines
fs/nfs/nfs4filelayoutdev.c +2 -0
@@ -219,6 +219,8 @@ decode_and_add_ds(__be32 **pp, struct inode *inode)
 		goto out_err;
 	}
 	buf = kmalloc(rlen + 1, GFP_KERNEL);
+	if (!buf)
+		goto out_err;
 	buf[rlen] = '\0';
 	memcpy(buf, r_addr, rlen);
 
it seems that r_addr is a string, then kstdup() is emulated here.

Quoted 2 lines

re,
wh

Not quite. kstrdup() requires that the argument be a NUL-terminated string. The above code doesn't.

Trond

Benny Halevy

2010-10-28 14:14 UTC #3 lore raw

Quoted 23 lines · Trond Myklebust

Quoted 22 lines · walter harms
Quoted 9 lines · Dan Carpenter
The decode_and_add_ds() should return NULL on failure.
Signed-off-by Dan Carpenter <[email protected]>
fs/nfs/nfs4filelayoutdev.c +0 -0
index 51fe64a..098113c 100644
fs/nfs/nfs4filelayoutdev.c decode_and_add_ds(__be32 **pp, struct inode *inode) · line 226
Quoted 8 lines
fs/nfs/nfs4filelayoutdev.c +2 -0
@@ -219,6 +219,8 @@ decode_and_add_ds(__be32 **pp, struct inode *inode)
 		goto out_err;
 	}
 	buf = kmalloc(rlen + 1, GFP_KERNEL);
+	if (!buf)
+		goto out_err;
 	buf[rlen] = '\0';
 	memcpy(buf, r_addr, rlen);
 
it seems that r_addr is a string, then kstdup() is emulated here.

Quoted 5 lines

Quoted 2 lines
re,
wh
Not quite. kstrdup() requires that the argument be a NUL-terminated string. The above code doesn't.

Right. kmemdup is the right one.

Benny

Quoted 6 lines

Trond
--
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

Dan Carpenter

author

2010-10-28 16:57 UTC #4 lore raw

Quoted 16 lines · Benny Halevy

Quoted 16 lines
Quoted 16 lines
fs/nfs/nfs4filelayoutdev.c decode_and_add_ds(__be32 **pp, struct inode *inode) · line 226
Quoted 13 lines
fs/nfs/nfs4filelayoutdev.c +2 -0
index 51fe64a..098113c 100644
@@ -219,6 +219,8 @@ decode_and_add_ds(__be32 **pp, struct inode *inode)
 		goto out_err;
 	}
 	buf = kmalloc(rlen + 1, GFP_KERNEL);
+	if (!buf)
+		goto out_err;
 	buf[rlen] = '\0';
 	memcpy(buf, r_addr, rlen);
 
it seems that r_addr is a string, then kstdup() is emulated here.

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
re,
wh
Not quite. kstrdup() requires that the argument be a NUL-terminated string. The above code doesn't.
Right. kmemdup is the right one.

We need to duplicate the data and also add a NUL char on the end. kmemdup() only does the first bit. You could copy one char past the end so you have space for the NUL but that's not the right idea.

Anyway, I'm out of here for the next few days. :) See you after the weekend.

regards,
dan carpenter

Benny Halevy

2010-10-29 07:33 UTC #5 lore raw

Quoted 20 lines · Dan Carpenter

Quoted 19 lines · Benny Halevy
Quoted 19 lines
Quoted 18 lines
fs/nfs/nfs4filelayoutdev.c decode_and_add_ds(__be32 **pp, struct inode *inode) · line 226
Quoted 13 lines
fs/nfs/nfs4filelayoutdev.c +2 -0
index 51fe64a..098113c 100644
@@ -219,6 +219,8 @@ decode_and_add_ds(__be32 **pp, struct inode *inode)
 		goto out_err;
 	}
 	buf = kmalloc(rlen + 1, GFP_KERNEL);
+	if (!buf)
+		goto out_err;
 	buf[rlen] = '\0';
 	memcpy(buf, r_addr, rlen);
 
it seems that r_addr is a string, then kstdup() is emulated here.
re,
wh

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
Not quite. kstrdup() requires that the argument be a NUL-terminated string. The above code doesn't.
Right. kmemdup is the right one.
We need to duplicate the data and also add a NUL char on the end.

You're right.

Quoted 2 lines

kmemdup() only does the first bit. You could copy one char past the end so you have space for the NUL but that's not the right idea.

If rlen is divisible by 4 you can't be sure you'll have an extra character to copy.

So the check you added is probably the simplest solution.

Benny

Quoted 7 lines

Anyway, I'm out of here for the next few days. :) See you after the weekend.
regards,
dan carpenter