From patchwork Tue Jan 17 01:10:49 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Qu Wenruo X-Patchwork-Id: 9519767 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 99D1D60244 for ; Tue, 17 Jan 2017 01:11:12 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 843D323B23 for ; Tue, 17 Jan 2017 01:11:12 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 78D8628494; Tue, 17 Jan 2017 01:11:12 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1F6EC23B23 for ; Tue, 17 Jan 2017 01:11:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750981AbdAQBLK (ORCPT ); Mon, 16 Jan 2017 20:11:10 -0500 Received: from cn.fujitsu.com ([59.151.112.132]:59737 "EHLO heian.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1750900AbdAQBLI (ORCPT ); Mon, 16 Jan 2017 20:11:08 -0500 X-IronPort-AV: E=Sophos;i="5.22,518,1449504000"; d="scan'208";a="14819775" Received: from unknown (HELO cn.fujitsu.com) ([10.167.33.5]) by heian.cn.fujitsu.com with ESMTP; 17 Jan 2017 09:10:59 +0800 Received: from G08CNEXCHPEKD01.g08.fujitsu.local (unknown [10.167.33.80]) by cn.fujitsu.com (Postfix) with ESMTP id 0F64B47AD1F4 for ; Tue, 17 Jan 2017 09:10:54 +0800 (CST) Received: from localhost.localdomain (10.167.226.34) by G08CNEXCHPEKD01.g08.fujitsu.local (10.167.33.89) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 17 Jan 2017 09:10:53 +0800 From: Qu Wenruo To: Subject: [PATCH 1/2] btrfs: raid56: Don't keep rbio for later steal Date: Tue, 17 Jan 2017 09:10:49 +0800 Message-ID: <20170117011050.18843-1-quwenruo@cn.fujitsu.com> X-Mailer: git-send-email 2.11.0 MIME-Version: 1.0 X-Originating-IP: [10.167.226.34] X-yoursite-MailScanner-ID: 0F64B47AD1F4.ACCEA X-yoursite-MailScanner: Found to be clean X-yoursite-MailScanner-From: quwenruo@cn.fujitsu.com Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Before this patch, btrfs raid56 will keep raid56 rbio even all its IO is done. This may save some time allocating rbio, but it can cause deadly use-after-free bug, for the following case: Original fs: 4 devices RAID5 Process A | Process B -------------------------------------------------------------------------- | Start device replace | Now the fs has 5 devices | devid 0: replace device | devid 1~4: old devices btrfs_map_bio() | |- __btrfs_map_block() | | bbio has 5 stripes | | including devid 0 | |- raid56_parity_write() | | raid_write_end_io() | |- rbio_orig_end_io() | |- unlock_stripe() | Keeps the old rbio for | later steal, which has | stripe for devid 0 | | Cancel device replace | Now the fs has 4 devices | devid 0 is freed Some IO happens | raid_write_end_io() | |- rbio_orig_end_io() | |- unlock_stripe() | |- steal_rbio() | Use old rbio, whose | bbio has freed devid 0| stripe | Any access to rbio->bbio will | cause general protection or NULL | pointer dereference | Such bug can already be triggered by fstests btrfs/069 for RAID5/6 profiles. Fix it by not keeping old rbio in unlock_stripe(), so we just free the finished rbio and rbio->bbio, so above problem wont' happen. Signed-off-by: Qu Wenruo --- fs/btrfs/raid56.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c index b8ffd9ea7499..41ced67165c0 100644 --- a/fs/btrfs/raid56.c +++ b/fs/btrfs/raid56.c @@ -774,7 +774,6 @@ static noinline void unlock_stripe(struct btrfs_raid_bio *rbio) int bucket; struct btrfs_stripe_hash *h; unsigned long flags; - int keep_cache = 0; bucket = rbio_bucket(rbio); h = rbio->fs_info->stripe_hash_table->table + bucket; @@ -786,19 +785,6 @@ static noinline void unlock_stripe(struct btrfs_raid_bio *rbio) spin_lock(&rbio->bio_list_lock); if (!list_empty(&rbio->hash_list)) { - /* - * if we're still cached and there is no other IO - * to perform, just leave this rbio here for others - * to steal from later - */ - if (list_empty(&rbio->plug_list) && - test_bit(RBIO_CACHE_BIT, &rbio->flags)) { - keep_cache = 1; - clear_bit(RBIO_RMW_LOCKED_BIT, &rbio->flags); - BUG_ON(!bio_list_empty(&rbio->bio_list)); - goto done; - } - list_del_init(&rbio->hash_list); atomic_dec(&rbio->refs); @@ -846,13 +832,11 @@ static noinline void unlock_stripe(struct btrfs_raid_bio *rbio) goto done_nolock; } } -done: spin_unlock(&rbio->bio_list_lock); spin_unlock_irqrestore(&h->lock, flags); done_nolock: - if (!keep_cache) - remove_rbio_from_cache(rbio); + remove_rbio_from_cache(rbio); } static void __free_raid_bio(struct btrfs_raid_bio *rbio)