[PATCH] btrfs: add tree checker for items in remap tree

[PATCH] btrfs: add tree checker for items in remap tree

[Mark Harmstone]

Add write-time checking of items in the remap tree, to catch errors
before they are written to disk.

We're checking:

* That remap items, remap backrefs, and identity remaps aren't written
  unless the REMAP_TREE incompat flag is set
* That identity remaps have a size of 0
* That remap items and remap backrefs have a size of sizeof(struct
  btrfs_remap_item)
* That the objectid for these items is aligned to the sector size
* That the offset for these items (i.e. the size of the remapping) isn't
  0 and is aligned to the sector size
* That objectid + offset doesn't overflow

Signed-off-by: Mark Harmstone <mark@harmstone.com>
---
 fs/btrfs/tree-checker.c | 70 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 495d1dd61e66c2..b19066e8ef522c 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -1879,6 +1879,71 @@ static int check_raid_stripe_extent(const struct extent_buffer *leaf,
 	return 0;
 }
 
+static int check_remap_key(const struct extent_buffer *leaf,
+			   const struct btrfs_key *key, int slot)
+{
+	u32 item_size = btrfs_item_size(leaf, slot);
+	const u32 sectorsize = leaf->fs_info->sectorsize;
+	u64 end;
+
+	if (unlikely(!btrfs_fs_incompat(leaf->fs_info, REMAP_TREE))) {
+		generic_err(leaf, slot,
+		"remap key type %u present but REMAP_TREE incompat bit unset",
+			    key->type);
+		return -EUCLEAN;
+	}
+
+	switch (key->type) {
+	case BTRFS_IDENTITY_REMAP_KEY:
+		if (unlikely(item_size != 0)) {
+			generic_err(leaf, slot,
+		"invalid item size for IDENTITY_REMAP, have %u expect 0",
+				    item_size);
+			return -EUCLEAN;
+		}
+	break;
+	case BTRFS_REMAP_KEY:
+	case BTRFS_REMAP_BACKREF_KEY:
+		if (unlikely(item_size != sizeof(struct btrfs_remap_item))) {
+			generic_err(leaf, slot,
+		"invalid item size for remap key type %u, have %u expect %zu",
+				    key->type, item_size,
+				    sizeof(struct btrfs_remap_item));
+			return -EUCLEAN;
+		}
+		break;
+	}
+
+	if (unlikely(key->offset == 0)) {
+		generic_err(leaf, slot,
+			    "invalid remap key length, have 0 expect nonzero");
+		return -EUCLEAN;
+	}
+
+	if (unlikely(!IS_ALIGNED(key->objectid, sectorsize))) {
+		generic_err(leaf, slot,
+		"invalid remap key objectid, have %llu expect aligned to %u",
+			    key->objectid, sectorsize);
+		return -EUCLEAN;
+	}
+
+	if (unlikely(!IS_ALIGNED(key->offset, sectorsize))) {
+		generic_err(leaf, slot,
+		"invalid remap key offset (length), have %llu expect aligned to %u",
+			    key->offset, sectorsize);
+		return -EUCLEAN;
+	}
+
+	if (unlikely(check_add_overflow(key->objectid, key->offset, &end))) {
+		generic_err(leaf, slot,
+	"remap key overflow, objectid %llu + offset %llu wraps",
+			    key->objectid, key->offset);
+		return -EUCLEAN;
+	}
+
+	return 0;
+}
+
 static int check_dev_extent_item(const struct extent_buffer *leaf,
 				 const struct btrfs_key *key,
 				 int slot,
@@ -2130,6 +2195,11 @@ static enum btrfs_tree_block_status check_leaf_item(struct extent_buffer *leaf,
 	case BTRFS_FREE_SPACE_BITMAP_KEY:
 		ret = check_free_space_bitmap(leaf, key, slot);
 		break;
+	case BTRFS_IDENTITY_REMAP_KEY:
+	case BTRFS_REMAP_KEY:
+	case BTRFS_REMAP_BACKREF_KEY:
+		ret = check_remap_key(leaf, key, slot);
+		break;
 	}
 
 	if (unlikely(ret))
-- 
2.52.0

Re: [PATCH] btrfs: add tree checker for items in remap tree

[David Sterba]

The subject should be: "btrfs: tree-checker: ..."

On Mon, Mar 23, 2026 at 05:25:47PM +0000, Mark Harmstone wrote:
> Add write-time checking of items in the remap tree, to catch errors
> before they are written to disk.
> 
> We're checking:
> 
> * That remap items, remap backrefs, and identity remaps aren't written
>   unless the REMAP_TREE incompat flag is set
> * That identity remaps have a size of 0
> * That remap items and remap backrefs have a size of sizeof(struct
>   btrfs_remap_item)
> * That the objectid for these items is aligned to the sector size
> * That the offset for these items (i.e. the size of the remapping) isn't
>   0 and is aligned to the sector size
> * That objectid + offset doesn't overflow

Nice.

> Signed-off-by: Mark Harmstone <mark@harmstone.com>

Reviewed-by: David Sterba <dsterba@suse.com>

> ---
>  fs/btrfs/tree-checker.c | 70 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 495d1dd61e66c2..b19066e8ef522c 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -1879,6 +1879,71 @@ static int check_raid_stripe_extent(const struct extent_buffer *leaf,
>  	return 0;
>  }
>  
> +static int check_remap_key(const struct extent_buffer *leaf,
> +			   const struct btrfs_key *key, int slot)
> +{
> +	u32 item_size = btrfs_item_size(leaf, slot);

This can be const as well.

> +	const u32 sectorsize = leaf->fs_info->sectorsize;
> +	u64 end;