From patchwork Fri Feb 3 08:20:23 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Qu Wenruo X-Patchwork-Id: 9553575 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 BF7D8604A7 for ; Fri, 3 Feb 2017 08:21:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AF51D28173 for ; Fri, 3 Feb 2017 08:21:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A3E3228427; Fri, 3 Feb 2017 08:21:25 +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 0D6EF28173 for ; Fri, 3 Feb 2017 08:21:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752727AbdBCIVW (ORCPT ); Fri, 3 Feb 2017 03:21:22 -0500 Received: from cn.fujitsu.com ([59.151.112.132]:11990 "EHLO heian.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1751866AbdBCIVU (ORCPT ); Fri, 3 Feb 2017 03:21:20 -0500 X-IronPort-AV: E=Sophos;i="5.22,518,1449504000"; d="scan'208";a="15257523" Received: from unknown (HELO cn.fujitsu.com) ([10.167.33.5]) by heian.cn.fujitsu.com with ESMTP; 03 Feb 2017 16:20:33 +0800 Received: from G08CNEXCHPEKD01.g08.fujitsu.local (unknown [10.167.33.80]) by cn.fujitsu.com (Postfix) with ESMTP id 730CC47B152D for ; Fri, 3 Feb 2017 16:20:33 +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; Fri, 3 Feb 2017 16:20:34 +0800 From: Qu Wenruo To: Subject: [PATCH 5/5] btrfs: replace: Use ref counts to avoid destroying target device when canceled Date: Fri, 3 Feb 2017 16:20:23 +0800 Message-ID: <20170203082023.3577-6-quwenruo@cn.fujitsu.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170203082023.3577-1-quwenruo@cn.fujitsu.com> References: <20170203082023.3577-1-quwenruo@cn.fujitsu.com> MIME-Version: 1.0 X-Originating-IP: [10.167.226.34] X-yoursite-MailScanner-ID: 730CC47B152D.AC28C 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 When dev-replace and scrub are run at the same time, dev-replace can be canceled by scrub. It's quite common for btrfs/069. The backtrace would be like: general protection fault: 0000 [#1] SMP Workqueue: btrfs-endio-raid56 btrfs_endio_raid56_helper [btrfs] RIP: 0010:[] [] generic_make_request_checks+0x198/0x5a0 Call Trace: [] ? generic_make_request+0xcf/0x290 [] generic_make_request+0x24/0x290 [] ? generic_make_request+0xcf/0x290 [] submit_bio+0x6e/0x120 [] ? page_in_rbio+0x4d/0x80 [btrfs] [] ? rbio_orig_end_io+0x80/0x80 [btrfs] [] finish_rmw+0x401/0x550 [btrfs] [] validate_rbio_for_rmw+0x36/0x40 [btrfs] [] raid_rmw_end_io+0x7d/0x90 [btrfs] [] bio_endio+0x56/0x60 [] end_workqueue_fn+0x3c/0x40 [btrfs] [] btrfs_scrubparity_helper+0xef/0x610 [btrfs] [] btrfs_endio_raid56_helper+0xe/0x10 [btrfs] [] process_one_work+0x2af/0x720 [] ? process_one_work+0x22b/0x720 [] worker_thread+0x4b/0x4f0 [] ? process_one_work+0x720/0x720 [] ? process_one_work+0x720/0x720 [] kthread+0xf3/0x110 [] ? kthread_park+0x60/0x60 [] ret_from_fork+0x27/0x40 While in that case, target device can be destroyed at cancel time, leading to a user-after-free bug: Process A (dev-replace) | Process B(scrub) ---------------------------------------------------------------------- |(Any RW is OK) |scrub_setup_recheck_block() ||- btrfs_map_sblock() | Got a bbio with tgtdev btrfs_dev_replace_finishing() | |- btrfs_destory_dev_replace_tgtdev()| |- call_rcu(free_device) | |- __free_device() | |- kfree(device) | | Scrub worker: | Access bbio->stripes[], which | contains tgtdev. | This triggers general protection. The bug is mostly obvious for RAID5/6 since raid56 choose to keep old rbio and rbio->bbio for later steal, this hugely enlarged the race window and makes it much easier to trigger the bug. This patch introduces 'tgtdev_refs' and 'tgtdev_wait' for btrfs_device to wait for all its user released the target device. Signed-off-by: Qu Wenruo --- fs/btrfs/dev-replace.c | 7 ++++++- fs/btrfs/volumes.c | 36 +++++++++++++++++++++++++++++++++++- fs/btrfs/volumes.h | 10 ++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c index 5de280b9ad73..794a6a0bedf2 100644 --- a/fs/btrfs/dev-replace.c +++ b/fs/btrfs/dev-replace.c @@ -558,7 +558,6 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info, rcu_str_deref(src_device->name), src_device->devid, rcu_str_deref(tgt_device->name)); - tgt_device->is_tgtdev_for_dev_replace = 0; tgt_device->devid = src_device->devid; src_device->devid = BTRFS_DEV_REPLACE_DEVID; memcpy(uuid_tmp, tgt_device->uuid, sizeof(uuid_tmp)); @@ -579,6 +578,12 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info, btrfs_dev_replace_unlock(dev_replace, 1); + /* + * Only change is_tgtdev_for_dev_replace flag after all its + * users get released. + */ + wait_target_device(tgt_device); + tgt_device->is_tgtdev_for_dev_replace = 0; btrfs_rm_dev_replace_blocked(fs_info); btrfs_rm_dev_replace_remove_srcdev(fs_info, src_device); diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 3c3c69c0eee4..84db9fb22b7d 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -2064,6 +2064,7 @@ void btrfs_destroy_dev_replace_tgtdev(struct btrfs_fs_info *fs_info, WARN_ON(!tgtdev); mutex_lock(&fs_info->fs_devices->device_list_mutex); + wait_target_device(tgtdev); btrfs_sysfs_rm_device_link(fs_info->fs_devices, tgtdev); if (tgtdev->bdev) @@ -2598,6 +2599,8 @@ int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info, device->is_tgtdev_for_dev_replace = 1; device->mode = FMODE_EXCL; device->dev_stats_valid = 1; + atomic_set(&device->tgtdev_refs, 0); + init_waitqueue_head(&device->tgtdev_wait); set_blocksize(device->bdev, 4096); device->fs_devices = fs_info->fs_devices; list_add(&device->dev_list, &fs_info->fs_devices->devices); @@ -2624,6 +2627,8 @@ void btrfs_init_dev_replace_tgtdev_for_resume(struct btrfs_fs_info *fs_info, tgtdev->sector_size = sectorsize; tgtdev->fs_info = fs_info; tgtdev->in_fs_metadata = 1; + atomic_set(&tgtdev->tgtdev_refs, 0); + init_waitqueue_head(&tgtdev->tgtdev_wait); } static noinline int btrfs_update_device(struct btrfs_trans_handle *trans, @@ -5302,6 +5307,32 @@ static struct btrfs_bio *alloc_btrfs_bio(int total_stripes, int real_stripes) return bbio; } +static void pin_bbio_target_device(struct btrfs_bio *bbio) +{ + int i; + + for (i = 0; i < bbio->num_stripes; i++) { + struct btrfs_device *device = bbio->stripes[i].dev; + + if (device->is_tgtdev_for_dev_replace) + atomic_inc(&device->tgtdev_refs); + } +} + +static void unpin_bbio_target_device(struct btrfs_bio *bbio) +{ + int i; + + for (i = 0; i < bbio->num_stripes; i++) { + struct btrfs_device *device = bbio->stripes[i].dev; + + if (device->is_tgtdev_for_dev_replace) { + atomic_dec(&device->tgtdev_refs); + wake_up(&device->tgtdev_wait); + } + } +} + void btrfs_get_bbio(struct btrfs_bio *bbio) { WARN_ON(!atomic_read(&bbio->refs)); @@ -5312,8 +5343,10 @@ void btrfs_put_bbio(struct btrfs_bio *bbio) { if (!bbio) return; - if (atomic_dec_and_test(&bbio->refs)) + if (atomic_dec_and_test(&bbio->refs)) { + unpin_bbio_target_device(bbio); kfree(bbio); + } } static int __btrfs_map_block(struct btrfs_fs_info *fs_info, @@ -5868,6 +5901,7 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, bbio->stripes[0].physical = physical_to_patch_in_first_stripe; bbio->mirror_num = map->num_stripes + 1; } + pin_bbio_target_device(bbio); out: if (dev_replace_is_ongoing) { btrfs_dev_replace_clear_lock_blocking(dev_replace); diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index 24ba6bc3ec34..702361182597 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -149,6 +149,10 @@ struct btrfs_device { /* Counter to record the change of device stats */ atomic_t dev_stats_ccnt; atomic_t dev_stat_values[BTRFS_DEV_STAT_VALUES_MAX]; + + /* To ensure we wait this target device before destroying it */ + atomic_t tgtdev_refs; + wait_queue_head_t tgtdev_wait; }; /* @@ -538,4 +542,10 @@ struct list_head *btrfs_get_fs_uuids(void); void btrfs_set_fs_info_ptr(struct btrfs_fs_info *fs_info); void btrfs_reset_fs_info_ptr(struct btrfs_fs_info *fs_info); +static inline void wait_target_device(struct btrfs_device *tgtdev) +{ + if (!tgtdev || !tgtdev->is_tgtdev_for_dev_replace) + return; + wait_event(tgtdev->tgtdev_wait, atomic_read(&tgtdev->tgtdev_refs) == 0); +} #endif