Skip to content

Commit

Permalink
fs/btrfs: fix a bug that U-boot fs btrfs implementation doesn't handl…
Browse files Browse the repository at this point in the history
…e NO_HOLE feature correctly

[BUG]
When passing a btrfs with NO_HOLE feature to U-boot, and if one file
contains holes, then the hash of the file is not correct in U-boot:

 # mkfs.btrfs -f test.img	# Since v5.15, mkfs defaults to NO_HOLES
 # mount test.img /mnt/btrfs
 # xfs_io -f -c "pwrite 0 4k" -c "pwrite 8k 4k" /mnt/btrfs/file
 # md5sum /mnt/btrfs/file
 277f3840b275c74d01e979ea9d75ac19  /mnt/btrfs/file
 # umount /mnt/btrfs
 # ./u-boot
 => host bind 0 /home/adam/test.img
 => ls host 0
 <   >      12288  Mon Dec 27 05:35:23 2021  file
 => load host 0 0x1000000 file
 12288 bytes read in 0 ms
 => md5sum 0x1000000 0x3000
 md5 for 01000000 ... 01002fff ==> 855ffdbe4d0ccc5acab92e1b5330e4c1

The md5sum doesn't match at all.

[CAUSE]
In U-boot btrfs implementation, the function btrfs_read_file() has the
following iteration for file extent iteration:

	/* Read the aligned part */
	while (cur < aligned_end) {
		ret = lookup_data_extent(root, &path, ino, cur, &next_offset);
		if (ret < 0)
			goto out;
		if (ret > 0) {
			/* No next, direct exit */
			if (!next_offset) {
				ret = 0;
				goto out;
			}
		}
		/* Read file extent */

But for NO_HOLES features, hole extents will not have any extent item
for it.
Thus if @Cur is at a hole, lookup_data_extent() will just return >0, and
update @next_offset.

But we still believe there is some data to read for @Cur for ret > 0
case, causing we read extent data from the next file extent.

This means, what we do for above NO_HOLES btrfs is:
- Read 4K data from disk to file offset [0, 4K)
  So far the data is still correct

- Read 4K data from disk to file offset [4K, 8K)
  We didn't skip the 4K hole, but read the data at file offset [8K, 12K)
  into file offset [4K, 8K).

  This causes the checksum mismatch.

[FIX]
Add extra check to skip to the next non-hole range after
lookup_data_extent().

Signed-off-by: Qu Wenruo <[email protected]>
  • Loading branch information
adam900710 authored and trini committed Jan 18, 2022
1 parent 1617165 commit 7c07543
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions fs/btrfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,14 @@ int btrfs_file_read(struct btrfs_root *root, u64 ino, u64 file_offset, u64 len,
ret = 0;
goto out;
}
/*
* Find a extent gap, mostly caused by NO_HOLE feature.
* Just to next offset directly.
*/
if (next_offset > cur) {
cur = next_offset;
continue;
}
}
fi = btrfs_item_ptr(path.nodes[0], path.slots[0],
struct btrfs_file_extent_item);
Expand Down

0 comments on commit 7c07543

Please sign in to comment.