From patchwork Tue Feb 17 18:44:15 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Omar Sandoval X-Patchwork-Id: 5840801 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 3C9049F373 for ; Tue, 17 Feb 2015 18:44:39 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 3873920165 for ; Tue, 17 Feb 2015 18:44:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4B76920155 for ; Tue, 17 Feb 2015 18:44:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752061AbbBQSoT (ORCPT ); Tue, 17 Feb 2015 13:44:19 -0500 Received: from mail-pa0-f47.google.com ([209.85.220.47]:40784 "EHLO mail-pa0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751596AbbBQSoS (ORCPT ); Tue, 17 Feb 2015 13:44:18 -0500 Received: by paceu11 with SMTP id eu11so8249999pac.7 for ; Tue, 17 Feb 2015 10:44:17 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=Zh/bNX/x+pSuUv/VDlR3qZGQrk2E4E+5HDfLAQg2Gzk=; b=OZot2FGnljUr9qJu76FKyzLcl2ROS/WbfuKO+Bm4EGSD/mkV0f3hxfPprNq9ubys9o vI/HhSOlv4tMfKTwigAaABXIoQYSFJq4pZpwT3TxOYr8qAAKGDXULE1jXKz0ZzByfqYC z/EXHkRyj0FVR380tvqtWsD15M1gdB6+/yj+GOdnERf4D60tJd2n13xN4A+cuwJtuAMU Wa6SqV/0/YlgRbF185QkU8J32YC/bEKtyQhaO3AnF1b+kelRAiZlbrEtOr2THkUGJMxs z2jYiqYMESlrjL2JdtdFQdiqtl55Kf5NpKyzcXF0gxNRtR1n017Pk4UpQsqBWq7eKS7S A6AA== X-Gm-Message-State: ALoCoQlf1q1Z0n0dk2B4GVbR6E2xSia2gyBP9qLi2goTa/JKdfSTNXwrhTe+XGsTYYguzFeoQYCT X-Received: by 10.66.141.5 with SMTP id rk5mr51638674pab.112.1424198657864; Tue, 17 Feb 2015 10:44:17 -0800 (PST) Received: from mew (c-76-104-211-44.hsd1.wa.comcast.net. [76.104.211.44]) by mx.google.com with ESMTPSA id ok1sm12632570pdb.53.2015.02.17.10.44.16 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 17 Feb 2015 10:44:16 -0800 (PST) Date: Tue, 17 Feb 2015 10:44:15 -0800 From: Omar Sandoval To: Chris Mason , Josef Bacik , David Sterba Cc: linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/3] btrfs: handle race on ENOMEM in alloc_extent_buffer Message-ID: <20150217184415.GA3297@mew> References: <4fcdbbd7d6dc95598323b46dcf5db4356cb7dee8.1424168589.git.osandov@osandov.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4fcdbbd7d6dc95598323b46dcf5db4356cb7dee8.1424168589.git.osandov@osandov.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Feb 17, 2015 at 02:51:08AM -0800, Omar Sandoval wrote: > Consider the following interleaving of overlapping calls to > alloc_extent_buffer: > > Call 1: > > - Successfully allocates a few pages with find_or_create_page > - find_or_create_page fails, goto free_eb > - Unlocks the allocated pages > > Call 2: > - Calls find_or_create_page and gets a page in call 1's extent_buffer > - Finds that the page is already associated with an extent_buffer > - Grabs a reference to the half-written extent_buffer and calls > mark_extent_buffer_accessed on it > > mark_extent_buffer_accessed will then try to call mark_page_accessed on > a null page and panic. > > The fix is to clear page->private of the half-written extent_buffer's > pages all at once while holding mapping->private_lock. > > Signed-off-by: Omar Sandoval > --- > fs/btrfs/extent_io.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > [snip] Actually, I just realized that there's a simpler fix. I can resend the whole series for easier merging once I get some review, but for now, here's what I'm talking about: btrfs: handle race on ENOMEM in alloc_extent_buffer Consider the following interleaving of overlapping calls to alloc_extent_buffer: Call 1: - Successfully allocates a few pages with find_or_create_page - find_or_create_page fails, goto free_eb - Unlocks the allocated pages Call 2: - Calls find_or_create_page and gets a page in call 1's extent_buffer - Finds that the page is already associated with an extent_buffer - Grabs a reference to the half-written extent_buffer and calls mark_extent_buffer_accessed on it mark_extent_buffer_accessed will then try to call mark_page_accessed on a null page and panic. The fix is to decrement the reference count on the half-written extent_buffer before unlocking the pages so call 2 won't use it. We also set exists = NULL in the case that we don't use exists to avoid accidentally returning a freed extent_buffer in an error case. Signed-off-by: Omar Sandoval --- fs/btrfs/extent_io.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 790dbae..6b3cd72 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -4850,6 +4850,7 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info, mark_extent_buffer_accessed(exists, p); goto free_eb; } + exists = NULL; /* * Do this so attach doesn't complain and we need to @@ -4913,12 +4914,12 @@ again: return eb; free_eb: + WARN_ON(!atomic_dec_and_test(&eb->refs)); for (i = 0; i < num_pages; i++) { if (eb->pages[i]) unlock_page(eb->pages[i]); } - WARN_ON(!atomic_dec_and_test(&eb->refs)); btrfs_release_extent_buffer(eb); return exists; }