From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tsutomu Itoh Subject: Re: [PATCH] Btrfs: check return value of kmalloc() Date: Wed, 20 Apr 2011 09:51:30 +0900 Message-ID: <4DAE2E12.7060002@jp.fujitsu.com> References: <201104190527.AA00014@T-ITOH1.jp.fujitsu.com> <20110419111240.GV31675@twin.jikos.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: linux-btrfs@vger.kernel.org, chris.mason@oracle.com To: David Sterba Return-path: In-Reply-To: <20110419111240.GV31675@twin.jikos.cz> List-ID: (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 >> --- >> 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 >