From patchwork Mon Aug 8 18:14:26 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Fasheh X-Patchwork-Id: 1046322 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p78IEVmI022927 for ; Mon, 8 Aug 2011 18:14:31 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751570Ab1HHSO2 (ORCPT ); Mon, 8 Aug 2011 14:14:28 -0400 Received: from cantor2.suse.de ([195.135.220.15]:34968 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751355Ab1HHSO2 (ORCPT ); Mon, 8 Aug 2011 14:14:28 -0400 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 03C2B8C061; Mon, 8 Aug 2011 20:14:26 +0200 (CEST) Date: Mon, 8 Aug 2011 11:14:26 -0700 From: Mark Fasheh To: linux-btrfs@vger.kernel.org Cc: chris.mason@oracle.com Subject: [PATCH] btrfs: Don't BUG_ON kzalloc error in btrfs_lookup_csums_range() Message-ID: <20110808181426.GD8482@wotan.suse.de> Reply-To: Mark Fasheh Mime-Version: 1.0 Content-Disposition: inline Organization: SUSE Labs User-Agent: Mutt/1.5.9i Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Mon, 08 Aug 2011 18:15:23 +0000 (UTC) Unfortunately it isn't enough to just exit here - the kzalloc() happens in a loop and the allocated items are added to a linked list whose head is passed in from the caller. To fix the BUG_ON() and also provide the semantic that the list passed in is only modified on success, I create function-local temporary list that we add items too. If no error is met, that list is spliced to the callers at the end of the function. Otherwise the list will be walked and all items freed before the error value is returned. I did a simple test on this patch by forcing an error at the kzalloc() point and verifying that when this hits (git clone seemed to exercise this), the function throws the proper error. Unfortunately but predictably, we later hit a BUG_ON(ret) type line that still hasn't been fixed up ;) Signed-off-by: Mark Fasheh --- fs/btrfs/file-item.c | 15 +++++++++++++-- 1 files changed, 13 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c index b910694..679fbff 100644 --- a/fs/btrfs/file-item.c +++ b/fs/btrfs/file-item.c @@ -284,6 +284,7 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end, struct btrfs_ordered_sum *sums; struct btrfs_sector_sum *sector_sum; struct btrfs_csum_item *item; + LIST_HEAD(tmplist); unsigned long offset; int ret; size_t size; @@ -358,7 +359,10 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end, MAX_ORDERED_SUM_BYTES(root)); sums = kzalloc(btrfs_ordered_sum_size(root, size), GFP_NOFS); - BUG_ON(!sums); + if (!sums) { + ret = -ENOMEM; + goto fail; + } sector_sum = sums->sums; sums->bytenr = start; @@ -380,12 +384,19 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end, offset += csum_size; sector_sum++; } - list_add_tail(&sums->list, list); + list_add_tail(&sums->list, &tmplist); } path->slots[0]++; } ret = 0; fail: + while (ret < 0 && !list_empty(&tmplist)) { + sums = list_entry(&tmplist, struct btrfs_ordered_sum, list); + list_del(&sums->list); + kfree(sums); + } + list_splice_tail(&tmplist, list); + btrfs_free_path(path); return ret; }