diff mbox series

btrfs: tree-checker: skip name hash check for image dump

Message ID 0163b37d7cdb31ed39e0eff2f61cdb4f3cd90272.1720137702.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: tree-checker: skip name hash check for image dump | expand

Commit Message

Qu Wenruo July 5, 2024, 12:02 a.m. UTC
[BUG]
For an image dumpped with "btrfs-image -s", the restored filesystem will
not pass tree-checker:

 BTRFS critical (device dm-5): corrupt leaf: root=1 block=32522240 slot=6 ino=6, name hash mismatch with key, have 0x0000000019e196de expect 0x000000008dbfc2d2
 BTRFS error (device dm-5): read time tree block corruption detected on logical 32522240 mirror 1

[CAUSE]
Btrfs-image with "-s" option would sanitize the filenames, thus it's
ensured to cause file names to mismatch their hash.

[FIX]
Since btrfs-image is mostly for debug purposes, we can afford it to be
an exception for tree-checker.

Just allow image dump to skip the name hash mismatch.

There will be a little more risk, as image dump can still be mounted RW,
but the existing error handling is good enough to handle the mismatched
hash.

Reported-by: Andrea Gelmini <andrea.gelmini@gmail.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/tree-checker.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Andrea Gelmini July 5, 2024, 6:22 a.m. UTC | #1
Il giorno ven 5 lug 2024 alle ore 02:02 Qu Wenruo <wqu@suse.com> ha scritto:
> Just allow image dump to skip the name hash mismatch.

Thanks a lot Qu!

> +                * mismatch since "-s" can generaete different names.

Just a stupid note: "generaete".

Thanks again,
Gelma
Andrea Gelmini July 6, 2024, 10:47 a.m. UTC | #2
Il giorno ven 5 lug 2024 alle ore 02:02 Qu Wenruo <wqu@suse.com> ha scritto:
> [FIX]
> Since btrfs-image is mostly for debug purposes, we can afford it to be
> an exception for tree-checker.

Sorry Qu, but I'm so stupid. I didn't make it out.

I mean. Trying different combination, running:
btrfs-image -s -c4 -t4 /dev/device dump.btrfs

both with patched and not patched kernel, I was unable to mount it:
a) not patched kernel complains about crc and so on;
b) patched kernel doesn't recognize the dump as a BTRFS filesystem. No
error, no complain, it doesn't mount.

Also, doing a simple:
string dump.btrfs

I read filename in clear.

If you want I can prepare images of what I used and data and so on.

Thanks again,
Gelma
Qu Wenruo July 7, 2024, 3:16 a.m. UTC | #3
在 2024/7/6 20:17, Andrea Gelmini 写道:
> Il giorno ven 5 lug 2024 alle ore 02:02 Qu Wenruo <wqu@suse.com> ha scritto:
>> [FIX]
>> Since btrfs-image is mostly for debug purposes, we can afford it to be
>> an exception for tree-checker.
>
> Sorry Qu, but I'm so stupid. I didn't make it out.
>
> I mean. Trying different combination, running:
> btrfs-image -s -c4 -t4 /dev/device dump.btrfs
>
> both with patched and not patched kernel, I was unable to mount it:
> a) not patched kernel complains about crc and so on;
> b) patched kernel doesn't recognize the dump as a BTRFS filesystem. No
> error, no complain, it doesn't mount.

Mind to provide the dmesg of the failed mount?

And just in case, "btrfs ins super dump" of that restored fs?

Furthermore, how did you mount the fs?

For "btrfs-image" dump, the result is not a mountable btrfs, you need to
restore it first:

  btrfs-image -r dump.btrfs btrfs.raw

Then mount the restored image:

  mount btrfs.raw <mnt>

>
> Also, doing a simple:
> string dump.btrfs
>
> I read filename in clear.

That's the problem though, as during my local tests, I also find that
"btrfs-image -s" doesn't really fill garbage for the dump.

That's indeed a bug and I'm still trying to fix it.

Thanks,
Qu
>
> If you want I can prepare images of what I used and data and so on.
>
> Thanks again,
> Gelma
>
diff mbox series

Patch

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 6388786fd8b5..6fd7ed46812a 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -630,10 +630,14 @@  static int check_dir_item(struct extent_buffer *leaf,
 
 		/*
 		 * Special check for XATTR/DIR_ITEM, as key->offset is name
-		 * hash, should match its name
+		 * hash, should match its name.
+		 * But if it's METADUMP (btrfs-image dump) then we allow hash
+		 * mismatch since "-s" can generaete different names.
 		 */
-		if (key->type == BTRFS_DIR_ITEM_KEY ||
-		    key->type == BTRFS_XATTR_ITEM_KEY) {
+		if ((key->type == BTRFS_DIR_ITEM_KEY ||
+		     key->type == BTRFS_XATTR_ITEM_KEY) &&
+		    !(btrfs_super_flags(leaf->fs_info->super_copy) &
+		      (BTRFS_SUPER_FLAG_METADUMP | BTRFS_SUPER_FLAG_METADUMP_V2))) {
 			char namebuf[max(BTRFS_NAME_LEN, XATTR_NAME_MAX)];
 
 			read_extent_buffer(leaf, namebuf,