Re: [patch] nfs: check kmalloc() return

[Benny Halevy <bhalevy@panasas.com>]

On 2010-10-28 18:57, Dan Carpenter wrote:
> On Thu, Oct 28, 2010 at 04:14:12PM +0200, Benny Halevy wrote:
>>>>> diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
>>>>> index 51fe64a..098113c 100644
>>>>> --- a/fs/nfs/nfs4filelayoutdev.c
>>>>> +++ b/fs/nfs/nfs4filelayoutdev.c
>>>>> @@ -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
>>>
>>> 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.

> 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

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