Re: [PATCH] Btrfs: check return value of kmalloc()

[Tsutomu Itoh <t-itoh@jp.fujitsu.com>]

(2011/04/19 20:12), David Sterba wrote:

Thanks for your review.

> Hi,
> 
> there are more unchecked kmallocs, in inode.c:btrfs_add_delayed_iput()
> 
> 2030        delayed = kmalloc(sizeof(*delayed), GFP_NOFS | __GFP_NOFAIL);

I think that it doesn't fail ordinary when __GFP_NOFAIL is specified...

> 
> and in extent-tree.c:relocate_one_extent()
> 
> 7992        new_extents = kmalloc(sizeof(*new_extents),
> 7993                                                 GFP_NOFS);
> 
> the value is checked later, new_extents is passed to get_new_locations
> and there it's checked, but no other callers pass potential NULL and the
> check fits here and can be dropped from get_new_locations; there's a
> little chance that get_new_locations will be able to succesfully
> allocate the same data a jiffy later.

Yes, therefore I did not check 'new_extents'.

Thanks,
Tsutomu


> 
> IMO the check can be safely added here and get_new_locations cleaned up
> later.
> 
> Feel free to fold the changes into your patch.
> 
> I did not find any more unchecked allocatinos.
> 
> 
> dave
> 
> 
> On Tue, Apr 19, 2011 at 02:27:08PM +0900, Tsutomu Itoh wrote:
>> The check on the return value of kmalloc() in inode.c is added.
>>
>> Signed-off-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
>> ---
>>  fs/btrfs/inode.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>> index a4157cf..c718d27 100644
>> --- a/fs/btrfs/inode.c
>> +++ b/fs/btrfs/inode.c
>> @@ -953,6 +953,7 @@ static int cow_file_range_async(struct inode *inode, struct page *locked_page,
>>  			 1, 0, NULL, GFP_NOFS);
>>  	while (start < end) {
>>  		async_cow = kmalloc(sizeof(*async_cow), GFP_NOFS);
>> +		BUG_ON(!async_cow);
>>  		async_cow->inode = inode;
>>  		async_cow->root = root;
>>  		async_cow->locked_page = locked_page;
>> @@ -5001,6 +5002,8 @@ static noinline int uncompress_inline(struct btrfs_path *path,
>>  	inline_size = btrfs_file_extent_inline_item_len(leaf,
>>  					btrfs_item_nr(leaf, path->slots[0]));
>>  	tmp = kmalloc(inline_size, GFP_NOFS);
>> +	if (!tmp)
>> +		return -ENOMEM;
>>  	ptr = btrfs_file_extent_inline_start(item);
>>  
>>  	read_extent_buffer(leaf, tmp, ptr, inline_size);
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>