[PATCH v2] btrfs: improve error reporting in lookup_inline_extent_backref

[PATCH v2] btrfs: improve error reporting in lookup_inline_extent_backref

[Nikolay Borisov]

When iterating the backrefs in an extent item if the ptr to the
'current' backref record goes beyond the extent item a warning is
generated and -ENOENT is returned. However what's more appropriate to
debug such cases would be to return EUCLEAN and also print identifying
information about the performed search as well as the current content of
the leaf containing the possibly corrupted extent item.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
V2:
 * Removed WARN_ON and instead introduced btrfs_crit as an error printing
 mechanism
 * Now prints a proper error message with information about the searched reference
 fs/btrfs/extent-tree.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 963160a0c393..5e29c2cee46e 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -895,7 +895,14 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
 	err = -ENOENT;
 	while (1) {
 		if (ptr >= end) {
-			WARN_ON(ptr > end);
+			if (ptr > end) {
+				err = -EUCLEAN;
+				btrfs_crit(fs_info,
+"overrun extent record at slot %lu [%llu BTRFS_EXTENT_ITEM_KEY %llu] while looking for inline extent for root %llu owner %llu offset %llu",
+				path->slots[0], bytenr, num_bytes,
+				root_objectid, root_objectid, owner, offset);
+				btrfs_print_leaf(path->nodes[0]);
+			}
 			break;
 		}
 		iref = (struct btrfs_extent_inline_ref *)ptr;
--
2.25.1

[PATCH v3] btrfs: improve error reporting in lookup_inline_extent_backref

[Nikolay Borisov]

When iterating the backrefs in an extent item if the ptr to the
'current' backref record goes beyond the extent item a warning is
generated and -ENOENT is returned. However what's more appropriate to
debug such cases would be to return EUCLEAN and also print identifying
information about the performed search as well as the current content of
the leaf containing the possibly corrupted extent item.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---

V3:
 * Fixed format for the btree slot
 * Removed redundant argument passed to format string

 fs/btrfs/extent-tree.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 963160a0c393..eaac79d8c0e9 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -895,7 +895,14 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
 	err = -ENOENT;
 	while (1) {
 		if (ptr >= end) {
-			WARN_ON(ptr > end);
+			if (ptr > end) {
+				err = -EUCLEAN;
+				btrfs_crit(fs_info,
+"overrun extent record at slot %d [%llu BTRFS_EXTENT_ITEM_KEY %llu] while looking for inline extent for root %llu owner %llu offset %llu",
+				path->slots[0], bytenr, num_bytes,
+				root_objectid, owner, offset);
+				btrfs_print_leaf(path->nodes[0]);
+			}
 			break;
 		}
 		iref = (struct btrfs_extent_inline_ref *)ptr;
--
2.25.1

Re: [PATCH v2] btrfs: improve error reporting in lookup_inline_extent_backref

[Filipe Manana]

On Wed, Apr 27, 2022 at 01:03:44PM +0300, Nikolay Borisov wrote:
> When iterating the backrefs in an extent item if the ptr to the
> 'current' backref record goes beyond the extent item a warning is
> generated and -ENOENT is returned. However what's more appropriate to
> debug such cases would be to return EUCLEAN and also print identifying
> information about the performed search as well as the current content of
> the leaf containing the possibly corrupted extent item.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> V2:
>  * Removed WARN_ON and instead introduced btrfs_crit as an error printing
>  mechanism
>  * Now prints a proper error message with information about the searched reference
>  fs/btrfs/extent-tree.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 963160a0c393..5e29c2cee46e 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -895,7 +895,14 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
>  	err = -ENOENT;
>  	while (1) {
>  		if (ptr >= end) {
> -			WARN_ON(ptr > end);
> +			if (ptr > end) {
> +				err = -EUCLEAN;
> +				btrfs_crit(fs_info,
> +"overrun extent record at slot %lu [%llu BTRFS_EXTENT_ITEM_KEY %llu] while looking for inline extent for root %llu owner %llu offset %llu",
> +				path->slots[0], bytenr, num_bytes,
> +				root_objectid, root_objectid, owner, offset);

Printing 'parent' is also needed in order to figure out what type of reference
we were looking for.

Also, %d can be used to print the slot, as path->slots is an int array.

> +				btrfs_print_leaf(path->nodes[0]);

The leaf should be dumped before printing the message, see:

https://btrfs.wiki.kernel.org/index.php/Development_notes#Output

Other than that, it looks fine.

Thanks.

> +			}
>  			break;
>  		}
>  		iref = (struct btrfs_extent_inline_ref *)ptr;
> --
> 2.25.1
>