[PATCH] btrfs: check for overlapping extent items in tree checker

[PATCH] btrfs: check for overlapping extent items in tree checker

[Josef Bacik]

We're seeing a weird problem in production where we have overlapping
extent items in the extent tree.  It's unclear where these are coming
from, and in debugging we realized there's no check in the tree checker
for this sort of problem.  Add a check to the tree-checker to make sure
that the extents do not overlap each other.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/tree-checker.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 9e0e0ae2288c..43f905ab0a18 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -1233,7 +1233,8 @@ static void extent_err(const struct extent_buffer *eb, int slot,
 }
 
 static int check_extent_item(struct extent_buffer *leaf,
-			     struct btrfs_key *key, int slot)
+			     struct btrfs_key *key, int slot,
+			     struct btrfs_key *prev_key)
 {
 	struct btrfs_fs_info *fs_info = leaf->fs_info;
 	struct btrfs_extent_item *ei;
@@ -1453,6 +1454,26 @@ static int check_extent_item(struct extent_buffer *leaf,
 			   total_refs, inline_refs);
 		return -EUCLEAN;
 	}
+
+	if ((prev_key->type == BTRFS_EXTENT_ITEM_KEY) ||
+	    (prev_key->type == BTRFS_METADATA_ITEM_KEY)) {
+		u64 prev_end = prev_key->objectid;
+
+		if (prev_key->type == BTRFS_METADATA_ITEM_KEY)
+			prev_end += fs_info->nodesize;
+		else
+			prev_end += prev_key->offset;
+
+		if (unlikely(prev_end > key->objectid)) {
+			extent_err(leaf, slot,
+	"previous extent [%llu %u %llu] overlaps current extent [%llu %u %llu]",
+				   prev_key->objectid, prev_key->type,
+				   prev_key->offset, key->objectid, key->type,
+				   key->offset);
+			return -EUCLEAN;
+		}
+	}
+
 	return 0;
 }
 
@@ -1621,7 +1642,7 @@ static int check_leaf_item(struct extent_buffer *leaf,
 		break;
 	case BTRFS_EXTENT_ITEM_KEY:
 	case BTRFS_METADATA_ITEM_KEY:
-		ret = check_extent_item(leaf, key, slot);
+		ret = check_extent_item(leaf, key, slot, prev_key);
 		break;
 	case BTRFS_TREE_BLOCK_REF_KEY:
 	case BTRFS_SHARED_DATA_REF_KEY:
-- 
2.26.3

Re: [PATCH] btrfs: check for overlapping extent items in tree checker

[Qu Wenruo]

On 2022/8/4 02:28, Josef Bacik wrote:
> We're seeing a weird problem in production where we have overlapping
> extent items in the extent tree.  It's unclear where these are coming
> from, and in debugging we realized there's no check in the tree checker
> for this sort of problem.  Add a check to the tree-checker to make sure
> that the extents do not overlap each other.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/tree-checker.c | 25 +++++++++++++++++++++++--
>   1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 9e0e0ae2288c..43f905ab0a18 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -1233,7 +1233,8 @@ static void extent_err(const struct extent_buffer *eb, int slot,
>   }
>
>   static int check_extent_item(struct extent_buffer *leaf,
> -			     struct btrfs_key *key, int slot)
> +			     struct btrfs_key *key, int slot,
> +			     struct btrfs_key *prev_key)
>   {
>   	struct btrfs_fs_info *fs_info = leaf->fs_info;
>   	struct btrfs_extent_item *ei;
> @@ -1453,6 +1454,26 @@ static int check_extent_item(struct extent_buffer *leaf,
>   			   total_refs, inline_refs);
>   		return -EUCLEAN;
>   	}
> +
> +	if ((prev_key->type == BTRFS_EXTENT_ITEM_KEY) ||
> +	    (prev_key->type == BTRFS_METADATA_ITEM_KEY)) {
> +		u64 prev_end = prev_key->objectid;
> +
> +		if (prev_key->type == BTRFS_METADATA_ITEM_KEY)
> +			prev_end += fs_info->nodesize;
> +		else
> +			prev_end += prev_key->offset;
> +
> +		if (unlikely(prev_end > key->objectid)) {
> +			extent_err(leaf, slot,
> +	"previous extent [%llu %u %llu] overlaps current extent [%llu %u %llu]",
> +				   prev_key->objectid, prev_key->type,
> +				   prev_key->offset, key->objectid, key->type,
> +				   key->offset);
> +			return -EUCLEAN;
> +		}
> +	}
> +
>   	return 0;
>   }
>
> @@ -1621,7 +1642,7 @@ static int check_leaf_item(struct extent_buffer *leaf,
>   		break;
>   	case BTRFS_EXTENT_ITEM_KEY:
>   	case BTRFS_METADATA_ITEM_KEY:
> -		ret = check_extent_item(leaf, key, slot);
> +		ret = check_extent_item(leaf, key, slot, prev_key);
>   		break;
>   	case BTRFS_TREE_BLOCK_REF_KEY:
>   	case BTRFS_SHARED_DATA_REF_KEY:

Re: [PATCH] btrfs: check for overlapping extent items in tree checker

[David Sterba]

On Wed, Aug 03, 2022 at 02:28:47PM -0400, Josef Bacik wrote:
> We're seeing a weird problem in production where we have overlapping
> extent items in the extent tree.  It's unclear where these are coming
> from, and in debugging we realized there's no check in the tree checker
> for this sort of problem.  Add a check to the tree-checker to make sure
> that the extents do not overlap each other.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Added to misc-next, thanks.

Re: [PATCH] btrfs: check for overlapping extent items in tree checker

[Christoph Anton Mitterer]

Hey.

On Wed, 2022-08-03 at 14:28 -0400, Josef Bacik wrote:
> We're seeing a weird problem in production where we have overlapping
> extent items in the extent tree.  It's unclear where these are coming
> from, and in debugging we realized there's no check in the tree
> checker
> for this sort of problem.  Add a check to the tree-checker to make
> sure
> that the extents do not overlap each other.

Is there a way to check whether one was affected by this? I mean pro-
actively on already existing data? scrubbing once one has a kernel with
a patch?


And I assume this could cause data corruption?


Cheers,
Chris.