All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miao Xie <miaox@cn.fujitsu.com>
To: <linux-btrfs@vger.kernel.org>
Subject: [PATCH v4 09/10] Btrfs, raid56: fix use-after-free problem in the final device replace procedure on raid56
Date: Tue, 2 Dec 2014 20:39:30 +0800	[thread overview]
Message-ID: <1417523971-15553-10-git-send-email-miaox@cn.fujitsu.com> (raw)
In-Reply-To: <1417523971-15553-1-git-send-email-miaox@cn.fujitsu.com>

The commit c404e0dc (Btrfs: fix use-after-free in the finishing
procedure of the device replace) fixed a use-after-free problem
which happened when removing the source device at the end of device
replace, but at that time, btrfs didn't support device replace
on raid56, so we didn't fix the problem on the raid56 profile.
Currently, we implemented device replace for raid56, so we need
kick that problem out before we enable that function for raid56.

The fix method is very simple, we just increase the bio per-cpu
counter before we submit a raid56 io, and decrease the counter
when the raid56 io ends.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
Changelog v3 -> v4:
- None.

Changelog v2 -> v3:
- New patch to fix undealt use-after-free problem of the source device
  in the final device replace procedure.

Changelog v1 -> v2:
- None.
---
 fs/btrfs/ctree.h       |  7 ++++++-
 fs/btrfs/dev-replace.c |  4 ++--
 fs/btrfs/raid56.c      | 41 ++++++++++++++++++++++++++++++++---------
 fs/btrfs/raid56.h      |  4 ++--
 fs/btrfs/scrub.c       |  2 +-
 fs/btrfs/volumes.c     |  7 ++-----
 6 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index fc73e86..3770f4c 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -4156,7 +4156,12 @@ int btrfs_scrub_progress(struct btrfs_root *root, u64 devid,
 /* dev-replace.c */
 void btrfs_bio_counter_inc_blocked(struct btrfs_fs_info *fs_info);
 void btrfs_bio_counter_inc_noblocked(struct btrfs_fs_info *fs_info);
-void btrfs_bio_counter_dec(struct btrfs_fs_info *fs_info);
+void btrfs_bio_counter_sub(struct btrfs_fs_info *fs_info, s64 amount);
+
+static inline void btrfs_bio_counter_dec(struct btrfs_fs_info *fs_info)
+{
+	btrfs_bio_counter_sub(fs_info, 1);
+}
 
 /* reada.c */
 struct reada_control {
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 91f6b8f..326919b 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -928,9 +928,9 @@ void btrfs_bio_counter_inc_noblocked(struct btrfs_fs_info *fs_info)
 	percpu_counter_inc(&fs_info->bio_counter);
 }
 
-void btrfs_bio_counter_dec(struct btrfs_fs_info *fs_info)
+void btrfs_bio_counter_sub(struct btrfs_fs_info *fs_info, s64 amount)
 {
-	percpu_counter_dec(&fs_info->bio_counter);
+	percpu_counter_sub(&fs_info->bio_counter, amount);
 
 	if (waitqueue_active(&fs_info->replace_wait))
 		wake_up(&fs_info->replace_wait);
diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index 7e6f239..44573bf 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -162,6 +162,8 @@ struct btrfs_raid_bio {
 	 */
 	int bio_list_bytes;
 
+	int generic_bio_cnt;
+
 	atomic_t refs;
 
 	atomic_t stripes_pending;
@@ -354,6 +356,7 @@ static void merge_rbio(struct btrfs_raid_bio *dest,
 {
 	bio_list_merge(&dest->bio_list, &victim->bio_list);
 	dest->bio_list_bytes += victim->bio_list_bytes;
+	dest->generic_bio_cnt += victim->generic_bio_cnt;
 	bio_list_init(&victim->bio_list);
 }
 
@@ -891,6 +894,10 @@ static void rbio_orig_end_io(struct btrfs_raid_bio *rbio, int err, int uptodate)
 {
 	struct bio *cur = bio_list_get(&rbio->bio_list);
 	struct bio *next;
+
+	if (rbio->generic_bio_cnt)
+		btrfs_bio_counter_sub(rbio->fs_info, rbio->generic_bio_cnt);
+
 	free_raid_bio(rbio);
 
 	while (cur) {
@@ -1775,6 +1782,7 @@ int raid56_parity_write(struct btrfs_root *root, struct bio *bio,
 	struct btrfs_raid_bio *rbio;
 	struct btrfs_plug_cb *plug = NULL;
 	struct blk_plug_cb *cb;
+	int ret;
 
 	rbio = alloc_rbio(root, bbio, raid_map, stripe_len);
 	if (IS_ERR(rbio)) {
@@ -1785,12 +1793,19 @@ int raid56_parity_write(struct btrfs_root *root, struct bio *bio,
 	rbio->bio_list_bytes = bio->bi_iter.bi_size;
 	rbio->operation = BTRFS_RBIO_WRITE;
 
+	btrfs_bio_counter_inc_noblocked(root->fs_info);
+	rbio->generic_bio_cnt = 1;
+
 	/*
 	 * don't plug on full rbios, just get them out the door
 	 * as quickly as we can
 	 */
-	if (rbio_is_full(rbio))
-		return full_stripe_write(rbio);
+	if (rbio_is_full(rbio)) {
+		ret = full_stripe_write(rbio);
+		if (ret)
+			btrfs_bio_counter_dec(root->fs_info);
+		return ret;
+	}
 
 	cb = blk_check_plugged(btrfs_raid_unplug, root->fs_info,
 			       sizeof(*plug));
@@ -1801,10 +1816,13 @@ int raid56_parity_write(struct btrfs_root *root, struct bio *bio,
 			INIT_LIST_HEAD(&plug->rbio_list);
 		}
 		list_add_tail(&rbio->plug_list, &plug->rbio_list);
+		ret = 0;
 	} else {
-		return __raid56_parity_write(rbio);
+		ret = __raid56_parity_write(rbio);
+		if (ret)
+			btrfs_bio_counter_dec(root->fs_info);
 	}
-	return 0;
+	return ret;
 }
 
 /*
@@ -2139,19 +2157,17 @@ cleanup:
  */
 int raid56_parity_recover(struct btrfs_root *root, struct bio *bio,
 			  struct btrfs_bio *bbio, u64 *raid_map,
-			  u64 stripe_len, int mirror_num, int hold_bbio)
+			  u64 stripe_len, int mirror_num, int generic_io)
 {
 	struct btrfs_raid_bio *rbio;
 	int ret;
 
 	rbio = alloc_rbio(root, bbio, raid_map, stripe_len);
 	if (IS_ERR(rbio)) {
-		__free_bbio_and_raid_map(bbio, raid_map, !hold_bbio);
+		__free_bbio_and_raid_map(bbio, raid_map, generic_io);
 		return PTR_ERR(rbio);
 	}
 
-	if (hold_bbio)
-		set_bit(RBIO_HOLD_BBIO_MAP_BIT, &rbio->flags);
 	rbio->operation = BTRFS_RBIO_READ_REBUILD;
 	bio_list_add(&rbio->bio_list, bio);
 	rbio->bio_list_bytes = bio->bi_iter.bi_size;
@@ -2159,11 +2175,18 @@ int raid56_parity_recover(struct btrfs_root *root, struct bio *bio,
 	rbio->faila = find_logical_bio_stripe(rbio, bio);
 	if (rbio->faila == -1) {
 		BUG();
-		__free_bbio_and_raid_map(bbio, raid_map, !hold_bbio);
+		__free_bbio_and_raid_map(bbio, raid_map, generic_io);
 		kfree(rbio);
 		return -EIO;
 	}
 
+	if (generic_io) {
+		btrfs_bio_counter_inc_noblocked(root->fs_info);
+		rbio->generic_bio_cnt = 1;
+	} else {
+		set_bit(RBIO_HOLD_BBIO_MAP_BIT, &rbio->flags);
+	}
+
 	/*
 	 * reconstruct from the q stripe if they are
 	 * asking for mirror 3
diff --git a/fs/btrfs/raid56.h b/fs/btrfs/raid56.h
index 3d4ddb3..31d4a15 100644
--- a/fs/btrfs/raid56.h
+++ b/fs/btrfs/raid56.h
@@ -43,8 +43,8 @@ struct btrfs_raid_bio;
 struct btrfs_device;
 
 int raid56_parity_recover(struct btrfs_root *root, struct bio *bio,
-				 struct btrfs_bio *bbio, u64 *raid_map,
-				 u64 stripe_len, int mirror_num, int hold_bbio);
+			  struct btrfs_bio *bbio, u64 *raid_map,
+			  u64 stripe_len, int mirror_num, int generic_io);
 int raid56_parity_write(struct btrfs_root *root, struct bio *bio,
 			       struct btrfs_bio *bbio, u64 *raid_map,
 			       u64 stripe_len);
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 1d6f16a..1807101 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -1478,7 +1478,7 @@ static int scrub_submit_raid56_bio_wait(struct btrfs_fs_info *fs_info,
 	ret = raid56_parity_recover(fs_info->fs_root, bio, page->recover->bbio,
 				    page->recover->raid_map,
 				    page->recover->map_length,
-				    page->mirror_num, 1);
+				    page->mirror_num, 0);
 	if (ret)
 		return ret;
 
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index b6c4730..d13b253 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5849,12 +5849,9 @@ int btrfs_map_bio(struct btrfs_root *root, int rw, struct bio *bio,
 		} else {
 			ret = raid56_parity_recover(root, bio, bbio,
 						    raid_map, map_length,
-						    mirror_num, 0);
+						    mirror_num, 1);
 		}
-		/*
-		 * FIXME, replace dosen't support raid56 yet, please fix
-		 * it in the future.
-		 */
+
 		btrfs_bio_counter_dec(root->fs_info);
 		return ret;
 	}
-- 
1.9.3


  parent reply	other threads:[~2014-12-02 12:38 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-02 12:39 [PATCH v4 00/10] Implement device scrub/replace for RAID56 Miao Xie
2014-12-02 12:39 ` [PATCH v4 01/10] Btrfs: remove noused bbio_ret in __btrfs_map_block in condition Miao Xie
2014-12-02 12:39 ` [PATCH v4 02/10] Btrfs: remove unnecessary code of stripe_index assignment in __btrfs_map_block Miao Xie
2014-12-02 12:39 ` [PATCH v4 03/10] Btrfs, raid56: don't change bbio and raid_map Miao Xie
2014-12-02 12:39 ` [PATCH v4 04/10] Btrfs, scrub: repair the common data on RAID5/6 if it is corrupted Miao Xie
2014-12-02 12:39 ` [PATCH v4 05/10] Btrfs, raid56: use a variant to record the operation type Miao Xie
2014-12-02 12:39 ` [PATCH v4 06/10] Btrfs, raid56: support parity scrub on raid56 Miao Xie
2014-12-02 12:39 ` [PATCH v4 07/10] Btrfs, replace: write dirty pages into the replace target device Miao Xie
2014-12-02 12:39 ` [PATCH v4 08/10] Btrfs, replace: write raid56 parity " Miao Xie
2014-12-02 12:39 ` Miao Xie [this message]
2014-12-02 12:39 ` [PATCH v4 10/10] Btrfs, replace: enable dev-replace for raid56 Miao Xie
2014-12-02 13:28 ` [PATCH v4 00/10] Implement device scrub/replace for RAID56 Chris Mason
2014-12-02 13:57   ` Wang Shilong
2014-12-03  1:19   ` Miao Xie
2014-12-03  2:29   ` Miao Xie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1417523971-15553-10-git-send-email-miaox@cn.fujitsu.com \
    --to=miaox@cn.fujitsu.com \
    --cc=linux-btrfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.