diff mbox series

[1/2] btrfs-progs: convert: fix bad csum for migrated range.

Message ID e311a44285872ad0903feeb1b69bdcab6f50a731.1683592875.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: convert: fix csum generation for migrated ranges | expand

Commit Message

Qu Wenruo May 9, 2023, 12:43 a.m. UTC
[BUG]
There is a report that btrfs-convert leads to bad csum for the image
file.

The reproducer looks like this:
(note the 64K block size, it's used to force a certain chunk layout)

 # touch test.img
 # truncate -s 10G test.img
 # mkfs.ext4 -b 64K test.img
 # btrfs-convert -N 64K test.img
 # btrfs check --check-data-csum test.img
 Opening filesystem to check...
 Checking filesystem on /home/adam/test.img
 UUID: 39d49537-a9f5-47f1-b6ab-7857707b9133
 [1/7] checking root items
 [2/7] checking extents
 [3/7] checking free space cache
 [4/7] checking fs roots
 [5/7] checking csums against data
 mirror 1 bytenr 4563140608 csum 0x3f1fa0ef expected csum 0xa4c4c072
 mirror 1 bytenr 4563206144 csum 0x55dcf0d3 expected csum 0xa4c4c072
 mirror 1 bytenr 4563271680 csum 0x4491b00a expected csum 0xa4c4c072
 mirror 1 bytenr 4563337216 csum 0x655d1f61 expected csum 0xa4c4c072
 mirror 1 bytenr 4563402752 csum 0xd37114d3 expected csum 0xa4c4c072
 mirror 1 bytenr 4563468288 csum 0x4c2dab30 expected csum 0xa4c4c072
 mirror 1 bytenr 4563533824 csum 0xa80fceed expected csum 0xa4c4c072
 mirror 1 bytenr 4563599360 csum 0xaf610db8 expected csum 0xa4c4c072
 mirror 1 bytenr 4563795968 csum 0x67b3c8a0 expected csum 0xa4c4c072
 ERROR: errors found in csum tree
 [6/7] checking root refs
 ...

[CAUSE]
Above initial failure is for logical bytenr of 4563140608, which is
inside the relocated range of the image file offset [0, 1M).

During convert, we migrate the original image file ranges which would
later be covered by super and other reserved ranges.

The migration happens
- Read out the original data
- Reserve a new file extent
- Write the data back to the file extent
  Note that, the new file extent can be inside some new data chunks,
  thus it's no longer 1:1 mapped.
- Generate the new csum for the new file extent

The problem happens at the last stage. We should read out the data from
the new file extent, but we call read_disk_extent() using the logical
bytenr, however read_disk_extent() is not doing logical -> physical
mapping.

Thus we will read some garbage, not the newly written data, and use
those garbage to generate csum. And caused the above problem.

[FIX]
Instead of read_disk_extent(), call read_data_from_disk(), which would
do the proper logical -> physical mapping, thus would fix the bug.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/main.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/convert/main.c b/convert/main.c
index 941b5ce3244d..46c6dfc571ff 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -191,10 +191,18 @@  static int csum_disk_extent(struct btrfs_trans_handle *trans,
 	if (!buffer)
 		return -ENOMEM;
 	for (offset = 0; offset < num_bytes; offset += blocksize) {
-		ret = read_disk_extent(root, disk_bytenr + offset,
-					blocksize, buffer);
+		u64 read_len = blocksize;
+
+		ret = read_data_from_disk(root->fs_info, buffer,
+					  disk_bytenr + offset, &read_len, 0);
 		if (ret)
 			break;
+		if (read_len == 0) {
+			error("failed to read logical bytenr %llu",
+			      disk_bytenr + offset);
+			ret = -EIO;
+			break;
+		}
 		ret = btrfs_csum_file_block(trans,
 					    disk_bytenr + num_bytes,
 					    disk_bytenr + offset,