Linux-BTRFS Archive on lore.kernel.org
 help / Atom feed
* [PATCH v2 0/2] Enhance btrfs_verify_dev_extents() to do more checks on dev extents
@ 2018-10-05  9:45 Qu Wenruo
  2018-10-05  9:45 ` [PATCH v2 1/2] btrfs: volumes: Make sure there is no overlap dev extents at mount time Qu Wenruo
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Qu Wenruo @ 2018-10-05  9:45 UTC (permalink / raw)
  To: linux-btrfs

Inspired by Hans' possible flawed DUP chunk allocator, add the following
dev extents checker:

1) Dev extent overlap check
   Dev extents don't use extent_cache so it can't report dev extents
   overlap.
   So manually check dev extents overlap.
   This check is pretty simple since we're already iterating dev extents
   by its physical offset, we only need to remember previous checked dev
   extents to do such check.

2) Dev extent end check
   No dev extent should go beyond device boundary.

These two checks are pretty cheap so it shouldn't bring any performance
overhead.

Changelog:
v2:
  Add "Link:" tag for the first patch.
  Move the actual check into verify_one_dev_extent() for the 2nd patch.

Qu Wenruo (2):
  btrfs: volumes: Make sure there is no overlap dev extents at mount
    time
  btrfs: volumes: Make sure no dev extent is beyond device boundary

 fs/btrfs/volumes.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

-- 
2.19.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] btrfs: volumes: Make sure there is no overlap dev extents at mount time
  2018-10-05  9:45 [PATCH v2 0/2] Enhance btrfs_verify_dev_extents() to do more checks on dev extents Qu Wenruo
@ 2018-10-05  9:45 ` Qu Wenruo
  2018-10-05  9:45 ` [PATCH v2 2/2] btrfs: volumes: Make sure no dev extent is beyond device boundary Qu Wenruo
  2018-11-05 15:15 ` [PATCH v2 0/2] Enhance btrfs_verify_dev_extents() to do more checks on dev extents David Sterba
  2 siblings, 0 replies; 5+ messages in thread
From: Qu Wenruo @ 2018-10-05  9:45 UTC (permalink / raw)
  To: linux-btrfs

Enhance btrfs_verify_dev_extents() to remember previous checked dev
extents, so it can verify no dev extents can overlap.

Link: https://www.spinics.net/lists/linux-btrfs/msg82370.html
Reported-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/volumes.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index da86706123ff..bf0b2c16847a 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -7462,6 +7462,8 @@ int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info)
 	struct btrfs_path *path;
 	struct btrfs_root *root = fs_info->dev_root;
 	struct btrfs_key key;
+	u64 prev_devid = 0;
+	u64 prev_dev_ext_end = 0;
 	int ret = 0;
 
 	key.objectid = 1;
@@ -7506,10 +7508,22 @@ int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info)
 		chunk_offset = btrfs_dev_extent_chunk_offset(leaf, dext);
 		physical_len = btrfs_dev_extent_length(leaf, dext);
 
+		/* Check if this dev extent over lap with previous one */
+		if (devid == prev_devid && physical_offset < prev_dev_ext_end) {
+			btrfs_err(fs_info,
+"dev extent devid %llu physical offset %llu overlap with previous dev extent end %llu",
+				  devid, physical_offset, prev_dev_ext_end);
+			ret = -EUCLEAN;
+			goto out;
+		}
+
 		ret = verify_one_dev_extent(fs_info, chunk_offset, devid,
 					    physical_offset, physical_len);
 		if (ret < 0)
 			goto out;
+		prev_devid = devid;
+		prev_dev_ext_end = physical_offset + physical_len;
+
 		ret = btrfs_next_item(root, path);
 		if (ret < 0)
 			goto out;
-- 
2.19.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] btrfs: volumes: Make sure no dev extent is beyond device boundary
  2018-10-05  9:45 [PATCH v2 0/2] Enhance btrfs_verify_dev_extents() to do more checks on dev extents Qu Wenruo
  2018-10-05  9:45 ` [PATCH v2 1/2] btrfs: volumes: Make sure there is no overlap dev extents at mount time Qu Wenruo
@ 2018-10-05  9:45 ` Qu Wenruo
  2019-01-07 16:27   ` Filipe Manana
  2018-11-05 15:15 ` [PATCH v2 0/2] Enhance btrfs_verify_dev_extents() to do more checks on dev extents David Sterba
  2 siblings, 1 reply; 5+ messages in thread
From: Qu Wenruo @ 2018-10-05  9:45 UTC (permalink / raw)
  To: linux-btrfs

Add extra dev extent end check against device boundary.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/volumes.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index bf0b2c16847a..bc3ac4715694 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -7371,6 +7371,7 @@ static int verify_one_dev_extent(struct btrfs_fs_info *fs_info,
 	struct extent_map_tree *em_tree = &fs_info->mapping_tree.map_tree;
 	struct extent_map *em;
 	struct map_lookup *map;
+	struct btrfs_device *dev;
 	u64 stripe_len;
 	bool found = false;
 	int ret = 0;
@@ -7420,6 +7421,22 @@ static int verify_one_dev_extent(struct btrfs_fs_info *fs_info,
 			physical_offset, devid);
 		ret = -EUCLEAN;
 	}
+
+	/* Make sure no dev extent is beyond device bondary */
+	dev = btrfs_find_device(fs_info, devid, NULL, NULL);
+	if (!dev) {
+		btrfs_err(fs_info, "failed to find devid %llu", devid);
+		ret = -EUCLEAN;
+		goto out;
+	}
+	if (physical_offset + physical_len > dev->disk_total_bytes) {
+		btrfs_err(fs_info,
+"dev extent devid %llu physical offset %llu len %llu is beyond device boundary %llu",
+			  devid, physical_offset, physical_len,
+			  dev->disk_total_bytes);
+		ret = -EUCLEAN;
+		goto out;
+	}
 out:
 	free_extent_map(em);
 	return ret;
-- 
2.19.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 0/2] Enhance btrfs_verify_dev_extents() to do more checks on dev extents
  2018-10-05  9:45 [PATCH v2 0/2] Enhance btrfs_verify_dev_extents() to do more checks on dev extents Qu Wenruo
  2018-10-05  9:45 ` [PATCH v2 1/2] btrfs: volumes: Make sure there is no overlap dev extents at mount time Qu Wenruo
  2018-10-05  9:45 ` [PATCH v2 2/2] btrfs: volumes: Make sure no dev extent is beyond device boundary Qu Wenruo
@ 2018-11-05 15:15 ` David Sterba
  2 siblings, 0 replies; 5+ messages in thread
From: David Sterba @ 2018-11-05 15:15 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Fri, Oct 05, 2018 at 05:45:53PM +0800, Qu Wenruo wrote:
> Inspired by Hans' possible flawed DUP chunk allocator, add the following
> dev extents checker:
> 
> 1) Dev extent overlap check
>    Dev extents don't use extent_cache so it can't report dev extents
>    overlap.
>    So manually check dev extents overlap.
>    This check is pretty simple since we're already iterating dev extents
>    by its physical offset, we only need to remember previous checked dev
>    extents to do such check.
> 
> 2) Dev extent end check
>    No dev extent should go beyond device boundary.
> 
> These two checks are pretty cheap so it shouldn't bring any performance
> overhead.
> 
> Changelog:
> v2:
>   Add "Link:" tag for the first patch.
>   Move the actual check into verify_one_dev_extent() for the 2nd patch.

Moved from for-next topic branch to misc-next, with some changelog
updates. Thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] btrfs: volumes: Make sure no dev extent is beyond device boundary
  2018-10-05  9:45 ` [PATCH v2 2/2] btrfs: volumes: Make sure no dev extent is beyond device boundary Qu Wenruo
@ 2019-01-07 16:27   ` Filipe Manana
  0 siblings, 0 replies; 5+ messages in thread
From: Filipe Manana @ 2019-01-07 16:27 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Fri, Oct 5, 2018 at 10:46 AM Qu Wenruo <wqu@suse.com> wrote:
>
> Add extra dev extent end check against device boundary.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/volumes.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index bf0b2c16847a..bc3ac4715694 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -7371,6 +7371,7 @@ static int verify_one_dev_extent(struct btrfs_fs_info *fs_info,
>         struct extent_map_tree *em_tree = &fs_info->mapping_tree.map_tree;
>         struct extent_map *em;
>         struct map_lookup *map;
> +       struct btrfs_device *dev;
>         u64 stripe_len;
>         bool found = false;
>         int ret = 0;
> @@ -7420,6 +7421,22 @@ static int verify_one_dev_extent(struct btrfs_fs_info *fs_info,
>                         physical_offset, devid);
>                 ret = -EUCLEAN;
>         }
> +
> +       /* Make sure no dev extent is beyond device bondary */
> +       dev = btrfs_find_device(fs_info, devid, NULL, NULL);

Qu,

This breaks use cases including seed devices. In those cases we need
to find a device by ID and FSID (not not return the first device we
find with a specific ID, ignoring the FSID).
Fstest btrfs/163 now fails in 5.0-rc1 because of this change.

Thanks.

> +       if (!dev) {
> +               btrfs_err(fs_info, "failed to find devid %llu", devid);
> +               ret = -EUCLEAN;
> +               goto out;
> +       }
> +       if (physical_offset + physical_len > dev->disk_total_bytes) {
> +               btrfs_err(fs_info,
> +"dev extent devid %llu physical offset %llu len %llu is beyond device boundary %llu",
> +                         devid, physical_offset, physical_len,
> +                         dev->disk_total_bytes);
> +               ret = -EUCLEAN;
> +               goto out;
> +       }
>  out:
>         free_extent_map(em);
>         return ret;
> --
> 2.19.0
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-05  9:45 [PATCH v2 0/2] Enhance btrfs_verify_dev_extents() to do more checks on dev extents Qu Wenruo
2018-10-05  9:45 ` [PATCH v2 1/2] btrfs: volumes: Make sure there is no overlap dev extents at mount time Qu Wenruo
2018-10-05  9:45 ` [PATCH v2 2/2] btrfs: volumes: Make sure no dev extent is beyond device boundary Qu Wenruo
2019-01-07 16:27   ` Filipe Manana
2018-11-05 15:15 ` [PATCH v2 0/2] Enhance btrfs_verify_dev_extents() to do more checks on dev extents David Sterba

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org linux-btrfs@archiver.kernel.org
	public-inbox-index linux-btrfs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox