diff mbox series

[1/4] btrfs-progs: convert: fix inline extent size for symbol link

Message ID aaaca1f01a1ff0bcd9afc71e7eae700d34904343.1725348299.git.wqu@suse.com (mailing list archive)
State New
Headers show
Series btrfs-progs: convert: fix the invalid regular extents for symbol links | expand

Commit Message

Qu Wenruo Sept. 3, 2024, 7:28 a.m. UTC
[BUG]
Sometimes test case btrfs/012 fails randomly, with the failure to read a
softlink:

     QA output created by 012
     Checking converted btrfs against the original one:
    -OK
    +readlink: Structure needs cleaning
     Checking saved ext2 image against the original one:
     OK

Furthermore, this will trigger a kernel error message:

 BTRFS critical (device dm-2): regular/prealloc extent found for non-regular inode 133081

[CAUSE]
For that specific inode 133081, the tree dump looks like this:

        item 127 key (133081 INODE_ITEM 0) itemoff 40984 itemsize 160
                generation 1 transid 1 size 4095 nbytes 4096
                block group 0 mode 120777 links 1 uid 0 gid 0 rdev 0
                sequence 0 flags 0x0(none)
        item 128 key (133081 INODE_REF 133080) itemoff 40972 itemsize 12
                index 2 namelen 2 name: l3
        item 129 key (133081 EXTENT_DATA 0) itemoff 40919 itemsize 53
                generation 4 type 1 (regular)
                extent data disk byte 2147483648 nr 38080512
                extent data offset 37974016 nr 4096 ram 38080512
                extent compression 0 (none)

Note that, the soft link inode size is 4095, just one by smaller than
the sector size, but its nbytes is 4096, larger than the upper limit of
inline extent.

Thus it results the creation of a regular extent, but for btrfs we do
not accept a soft link with a regular/preallocated extent, thus kernel
rejects such read and failed the readlink call.

The root cause is in the convert code, where for soft links we always
create a data extent with its size + 1, causing the above problem.

I guess the original code is to handle the terminating NUL, but in btrfs
we never need to bother terminating NUL for inline extents.

Thus this pitfall in btrfs-convert leads to the above invalid data
extent and fail the test case.

[FIX]
Fix the corner case by removing the terminating NUL when inserting an
inline extent.

Furthermore, to prevent such problem from happening again, do extra
UASSERT() for btrfs_insert_inline_extent() to detect such problem.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/source-ext2.c     | 12 +++++++++---
 convert/source-reiserfs.c |  4 ++--
 kernel-shared/file-item.c |  3 +++
 3 files changed, 14 insertions(+), 5 deletions(-)
diff mbox series

Patch

diff --git a/convert/source-ext2.c b/convert/source-ext2.c
index acba5db7ee81..dfad64a6d226 100644
--- a/convert/source-ext2.c
+++ b/convert/source-ext2.c
@@ -476,8 +476,14 @@  static int ext2_create_symlink(struct btrfs_trans_handle *trans,
 	int ret;
 	char *pathname;
 	u64 inode_size = btrfs_stack_inode_size(btrfs_inode);
+
 	if (ext2fs_inode_data_blocks2(ext2_fs, ext2_inode)) {
-		btrfs_set_stack_inode_size(btrfs_inode, inode_size + 1);
+		if (inode_size >= trans->fs_info->sectorsize) {
+			error("inode size too large for symbol link, has %llu expect [1, %u]",
+				inode_size,  trans->fs_info->sectorsize - 1);
+			return -EUCLEAN;
+		}
+		btrfs_set_stack_inode_size(btrfs_inode, inode_size);
 		ret = ext2_create_file_extents(trans, root, objectid,
 				btrfs_inode, ext2_fs, ext2_ino,
 				CONVERT_FLAG_DATACSUM |
@@ -489,8 +495,8 @@  static int ext2_create_symlink(struct btrfs_trans_handle *trans,
 	pathname = (char *)&(ext2_inode->i_block[0]);
 	BUG_ON(pathname[inode_size] != 0);
 	ret = btrfs_insert_inline_extent(trans, root, objectid, 0,
-					 pathname, inode_size + 1);
-	btrfs_set_stack_inode_nbytes(btrfs_inode, inode_size + 1);
+					 pathname, inode_size);
+	btrfs_set_stack_inode_nbytes(btrfs_inode, inode_size);
 	return ret;
 }
 
diff --git a/convert/source-reiserfs.c b/convert/source-reiserfs.c
index 3edc72ed08a5..e2f07bfda188 100644
--- a/convert/source-reiserfs.c
+++ b/convert/source-reiserfs.c
@@ -538,8 +538,8 @@  static int reiserfs_copy_symlink(struct btrfs_trans_handle *trans,
 	len = get_ih_item_len(tp_item_head(&path));
 
 	ret = btrfs_insert_inline_extent(trans, root, objectid, 0,
-					 symlink, len + 1);
-	btrfs_set_stack_inode_nbytes(btrfs_inode, len + 1);
+					 symlink, len);
+	btrfs_set_stack_inode_nbytes(btrfs_inode, len);
 fail:
 	pathrelse(&path);
 	return ret;
diff --git a/kernel-shared/file-item.c b/kernel-shared/file-item.c
index d2da56e1f523..22fa1b03bed7 100644
--- a/kernel-shared/file-item.c
+++ b/kernel-shared/file-item.c
@@ -26,6 +26,7 @@ 
 #include "kernel-shared/extent_io.h"
 #include "kernel-shared/uapi/btrfs.h"
 #include "common/internal.h"
+#include "common/messages.h"
 
 #define MAX_CSUM_ITEMS(r, size) ((((BTRFS_LEAF_DATA_SIZE(r->fs_info) - \
 			       sizeof(struct btrfs_item) * 2) / \
@@ -97,6 +98,8 @@  int btrfs_insert_inline_extent(struct btrfs_trans_handle *trans,
 	int err = 0;
 	int ret;
 
+	UASSERT(size < trans->fs_info->sectorsize);
+
 	path = btrfs_alloc_path();
 	if (!path)
 		return -ENOMEM;