[PATCH v2 0/7] Fix long standing -EOPNOTSUPP problem caused by large inline extent

[PATCH v2 0/7] Fix long standing -EOPNOTSUPP problem caused by large inline extent

[Qu Wenruo]

The patch is based on v4.15.1, and is designed to replace the old patch
in devel branch.

Kernel doesn't support dropping range inside inline extent, and prevents
such thing happening by limiting max inline extent size to
min(max_inline, sectorsize - 1) in cow_file_range_inline().

However btrfs-progs only inherit the BTRFS_MAX_INLINE_DATA_SIZE() macro,
which doesn't have sectorsize check.
And since btrfs-progs defaults to 16K nodesize, above macro allows large
inline extent over 15K size.

This leads to unexpected kernel behavior.

The bug exists in several parts of btrfs-progs, any tool which creates
file extent is involved, including:
1) btrfs-convert
2) mkfs --rootdir

This patchset fixes the problems in convert (both ext2 and reiserfs),
mkfs --rootdir, then add check support for both original and lowmem
mode, and finally adds 2 test cases, one for mkfs and one for convert.

For mkfs test case, it can already be exposed by misc/002, but a
pin-point test case will be much better.

Tested with test-convert, test-fsck, test-misc and test-mkfs.

Qu Wenruo (7):
  btrfs-progs: convert/ext2: Fix inline extent creation check
  btrfs-progs: convert/reiserfs: Fix inline file extent creation check
  btrfs-progs: mkfs/rootdir: Fix inline extent creation check
  btrfs-progs: check/original mode: Check inline extent size
  btrfs-progs: check/lowmem mode: Check inline extent size
  btrfs-progs: test/convert: Add test case for invalid large inline data
    extent
  btrfs-progs: test/mkfs: Add test case for rootdir inline extent size

 check/main.c                                       | 16 +++++
 check/mode-lowmem.c                                | 28 ++++++++
 check/mode-original.h                              |  1 +
 convert/source-ext2.c                              |  2 +-
 convert/source-reiserfs.c                          |  3 +-
 mkfs/rootdir.c                                     |  6 +-
 .../016-invalid-large-inline-extent/test.sh        | 22 ++++++
 tests/mkfs-tests/014-rootdir-inline-extent/test.sh | 81 ++++++++++++++++++++++
 8 files changed, 155 insertions(+), 4 deletions(-)
 create mode 100755 tests/convert-tests/016-invalid-large-inline-extent/test.sh
 create mode 100755 tests/mkfs-tests/014-rootdir-inline-extent/test.sh

-- 
2.16.2

[PATCH v2 1/7] btrfs-progs: convert/ext2: Fix inline extent creation check

[Qu Wenruo]

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/source-ext2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/convert/source-ext2.c b/convert/source-ext2.c
index b1492c78693d..e13fbe00415a 100644
--- a/convert/source-ext2.c
+++ b/convert/source-ext2.c
@@ -310,7 +310,7 @@ static int ext2_create_file_extents(struct btrfs_trans_handle *trans,
 	if (ret)
 		goto fail;
 	if ((convert_flags & CONVERT_FLAG_INLINE_DATA) && data.first_block == 0
-	    && data.num_blocks > 0
+	    && data.num_blocks > 0 && inode_size < sectorsize
 	    && inode_size <= BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info)) {
 		u64 num_bytes = data.num_blocks * sectorsize;
 		u64 disk_bytenr = data.disk_block * sectorsize;
-- 
2.16.2

[PATCH v2 2/7] btrfs-progs: convert/reiserfs: Fix inline file extent creation check

[Qu Wenruo]

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/source-reiserfs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/convert/source-reiserfs.c b/convert/source-reiserfs.c
index 39d6f0728bd3..eeb68d962c5d 100644
--- a/convert/source-reiserfs.c
+++ b/convert/source-reiserfs.c
@@ -376,7 +376,8 @@ static int reiserfs_convert_tail(struct btrfs_trans_handle *trans,
 	u64 isize;
 	int ret;
 
-	if (length >= BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info))
+	if (length >= BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info) ||
+	    length >= root->fs_info->sectorsize)
 		return convert_direct(trans, root, objectid, inode, body,
 				      length, offset, convert_flags);
 
-- 
2.16.2

[PATCH v2 3/7] btrfs-progs: mkfs/rootdir: Fix inline extent creation check

[Qu Wenruo]

Just like convert, we need extra check against sector size for creating
inline extent.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 mkfs/rootdir.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mkfs/rootdir.c b/mkfs/rootdir.c
index e06b65ac13e4..a1d223a2408a 100644
--- a/mkfs/rootdir.c
+++ b/mkfs/rootdir.c
@@ -139,7 +139,8 @@ static int fill_inode_item(struct btrfs_trans_handle *trans,
 	}
 	if (S_ISREG(src->st_mode)) {
 		btrfs_set_stack_inode_size(dst, (u64)src->st_size);
-		if (src->st_size <= BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info))
+		if (src->st_size <= BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info) &&
+		    src->st_size < sectorsize)
 			btrfs_set_stack_inode_nbytes(dst, src->st_size);
 		else {
 			blocks = src->st_size / sectorsize;
@@ -327,7 +328,8 @@ static int add_file_items(struct btrfs_trans_handle *trans,
 	if (st->st_size % sectorsize)
 		blocks += 1;
 
-	if (st->st_size <= BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info)) {
+	if (st->st_size <= BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info) &&
+	    st->st_size < sectorsize) {
 		char *buffer = malloc(st->st_size);
 
 		if (!buffer) {
-- 
2.16.2

[PATCH v2 4/7] btrfs-progs: check/original mode: Check inline extent size

[Qu Wenruo]

For inline compressed file extent, kernel doesn't allow inline extent
ram size larger than sector size and on-disk inline extent size should
not exceed BTRFS_MAX_INLINE_DATA_SIZE().

For inline uncompressed file extent, kernel doesn't allow inline extent
ram and on-disk size larger than either BTRFS_MAX_INLINE_DATA_SIZE() or
sector size.

Check it in original mode.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c          | 16 ++++++++++++++++
 check/mode-original.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/check/main.c b/check/main.c
index 97baae583f04..7821faa929a3 100644
--- a/check/main.c
+++ b/check/main.c
@@ -560,6 +560,8 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
 		fprintf(stderr, ", bad file extent");
 	if (errors & I_ERR_FILE_EXTENT_OVERLAP)
 		fprintf(stderr, ", file extent overlap");
+	if (errors & I_ERR_FILE_EXTENT_TOO_LARGE)
+		fprintf(stderr, ", inline file extent too large");
 	if (errors & I_ERR_FILE_EXTENT_DISCOUNT)
 		fprintf(stderr, ", file extent discount");
 	if (errors & I_ERR_DIR_ISIZE_WRONG)
@@ -1433,6 +1435,8 @@ static int process_file_extent(struct btrfs_root *root,
 	u64 disk_bytenr = 0;
 	u64 extent_offset = 0;
 	u64 mask = root->fs_info->sectorsize - 1;
+	u32 max_inline_size = min_t(u32, mask,
+				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
 	int extent_type;
 	int ret;
 
@@ -1458,9 +1462,21 @@ static int process_file_extent(struct btrfs_root *root,
 	extent_type = btrfs_file_extent_type(eb, fi);
 
 	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
+		u8 compression = btrfs_file_extent_compression(eb, fi);
+		struct btrfs_item *item = btrfs_item_nr(slot);
+
 		num_bytes = btrfs_file_extent_inline_len(eb, slot, fi);
 		if (num_bytes == 0)
 			rec->errors |= I_ERR_BAD_FILE_EXTENT;
+		if (compression) {
+			if (btrfs_file_extent_inline_item_len(eb, item) >
+			    max_inline_size ||
+			    num_bytes > root->fs_info->sectorsize)
+				rec->errors |= I_ERR_FILE_EXTENT_TOO_LARGE;
+		} else {
+			if (num_bytes > max_inline_size)
+				rec->errors |= I_ERR_FILE_EXTENT_TOO_LARGE;
+		}
 		rec->found_size += num_bytes;
 		num_bytes = (num_bytes + mask) & ~mask;
 	} else if (extent_type == BTRFS_FILE_EXTENT_REG ||
diff --git a/check/mode-original.h b/check/mode-original.h
index f859af478f0f..368de692fdd1 100644
--- a/check/mode-original.h
+++ b/check/mode-original.h
@@ -185,6 +185,7 @@ struct file_extent_hole {
 #define I_ERR_SOME_CSUM_MISSING		(1 << 12)
 #define I_ERR_LINK_COUNT_WRONG		(1 << 13)
 #define I_ERR_FILE_EXTENT_ORPHAN	(1 << 14)
+#define I_ERR_FILE_EXTENT_TOO_LARGE	(1 << 15)
 
 struct inode_record {
 	struct list_head backrefs;
-- 
2.16.2

[PATCH v2 5/7] btrfs-progs: check/lowmem mode: Check inline extent size

[Qu Wenruo]

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-lowmem.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 62bcf3d2e126..0d19b373c7af 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1417,6 +1417,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
 	u64 csum_found;		/* In byte size, sectorsize aligned */
 	u64 search_start;	/* Logical range start we search for csum */
 	u64 search_len;		/* Logical range len we search for csum */
+	u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
+				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
 	unsigned int extent_type;
 	unsigned int is_hole;
 	int compressed = 0;
@@ -1440,6 +1442,32 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
 				root->objectid, fkey->objectid, fkey->offset);
 			err |= FILE_EXTENT_ERROR;
 		}
+		if (compressed) {
+			if (extent_num_bytes > root->fs_info->sectorsize) {
+				error(
+"root %llu EXTENT_DATA[%llu %llu] too large inline extent ram size, have %llu, max: %u",
+					root->objectid, fkey->objectid,
+					fkey->offset, extent_num_bytes,
+					root->fs_info->sectorsize);
+				err |= FILE_EXTENT_ERROR;
+			}
+			if (item_inline_len > max_inline_extent_size) {
+				error(
+"root %llu EXTENT_DATA[%llu %llu] too large inline extent on-disk size, have %u, max: %u",
+					root->objectid, fkey->objectid,
+					fkey->offset, item_inline_len,
+					max_inline_extent_size);
+				err |= FILE_EXTENT_ERROR;
+			}
+		} else {
+			if (extent_num_bytes > max_inline_extent_size) {
+ 			error(
+ "root %llu EXTENT_DATA[%llu %llu] too large inline extent size, have %llu, max: %u",
+ 				root->objectid, fkey->objectid, fkey->offset,
+ 				extent_num_bytes, max_inline_extent_size);
+				err |= FILE_EXTENT_ERROR;
+			}
+		}
 		if (!compressed && extent_num_bytes != item_inline_len) {
 			error(
 		"root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected: %u",
-- 
2.16.2

[PATCH v2 6/7] btrfs-progs: test/convert: Add test case for invalid large inline data extent

[Qu Wenruo]

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 .../016-invalid-large-inline-extent/test.sh        | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100755 tests/convert-tests/016-invalid-large-inline-extent/test.sh

diff --git a/tests/convert-tests/016-invalid-large-inline-extent/test.sh b/tests/convert-tests/016-invalid-large-inline-extent/test.sh
new file mode 100755
index 000000000000..f37c7c09d2e7
--- /dev/null
+++ b/tests/convert-tests/016-invalid-large-inline-extent/test.sh
@@ -0,0 +1,22 @@
+#!/bin/bash
+# Check if btrfs-convert refuses to rollback the filesystem, and leave the fs
+# and the convert image untouched
+
+source "$TEST_TOP/common"
+source "$TEST_TOP/common.convert"
+
+setup_root_helper
+prepare_test_dev
+check_prereq btrfs-convert
+check_global_prereq mke2fs
+
+convert_test_prep_fs ext4 mke2fs -t ext4 -b 4096
+
+# Create a 6K file, which should not be inlined
+run_check $SUDO_HELPER dd if=/dev/zero bs=2k count=3 of="$TEST_MNT/file1"
+
+run_check_umount_test_dev
+
+# convert_test_do_convert() will call btrfs check, which should expose any
+# invalid inline extent with too large size
+convert_test_do_convert
-- 
2.16.2

[PATCH v2 7/7] btrfs-progs: test/mkfs: Add test case for rootdir inline extent size

[Qu Wenruo]

Add a test case for mkfs --rootdir, using files with different file
sizes to check if invalid large inline extent could exist.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/mkfs-tests/014-rootdir-inline-extent/test.sh | 81 ++++++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100755 tests/mkfs-tests/014-rootdir-inline-extent/test.sh

diff --git a/tests/mkfs-tests/014-rootdir-inline-extent/test.sh b/tests/mkfs-tests/014-rootdir-inline-extent/test.sh
new file mode 100755
index 000000000000..e0765b0bc4e2
--- /dev/null
+++ b/tests/mkfs-tests/014-rootdir-inline-extent/test.sh
@@ -0,0 +1,81 @@
+#!/bin/bash
+# Regression test for mkfs.btrfs --rootdir with inline file extents
+# For any large inline file extent, btrfs check could already report it
+
+source "$TOP/tests/common"
+
+check_global_prereq fallocate
+check_prereq mkfs.btrfs
+
+prepare_test_dev
+
+tmp=$(mktemp -d --tmpdir btrfs-progs-mkfs.rootdirXXXXXXX)
+
+pagesize=$(getconf PAGESIZE)
+create_file()
+{
+	local size=$1
+	# Reuse size and filename
+	run_check fallocate -l $size "$tmp/$size"
+}
+
+test_mkfs_rootdir()
+{
+	nodesize=$1
+	run_check "$TOP/mkfs.btrfs" --nodesize $nodesize -f --rootdir "$tmp" \
+		"$TEST_DEV"
+	run_check $SUDO_HELPER "$TOP/btrfs" check "$TEST_DEV"
+}
+
+# File sizes is designed to cross differnet node size, so even
+# the sectorsize is not 4K, we can still test it well.
+create_file 512
+create_file 1024
+create_file 2048
+
+create_file 3994
+create_file 3995	# For 4K node size, max inline would be 4k - 101
+create_file 3996
+
+create_file 4095
+create_file 4096
+create_file 4097
+
+create_file 8090
+create_file 8091
+create_file 8092
+
+create_file 8191
+create_file 8192
+create_file 8193
+
+create_file 16282
+create_file 16283
+create_file 16284
+
+create_file 16383
+create_file 16384
+create_file 16385
+
+create_file 32666
+create_file 32667
+create_file 32668
+
+create_file 32767
+create_file 32768
+create_file 32769
+
+create_file 65434
+create_file 65435
+create_file 65436
+
+create_file 65535
+create_file 65536
+create_file 65537
+
+for nodesize in 4096 8192 16384 32768 65536; do
+	if [ $nodesize -ge $pagesize ]; then
+		test_mkfs_rootdir $nodesize
+	fi
+done
+rm -rf -- "$tmp"
-- 
2.16.2

Re: [PATCH v2 5/7] btrfs-progs: check/lowmem mode: Check inline extent size

[Nikolay Borisov]

On 13.03.2018 03:56, Qu Wenruo wrote:
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  check/mode-lowmem.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 62bcf3d2e126..0d19b373c7af 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1417,6 +1417,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
>  	u64 csum_found;		/* In byte size, sectorsize aligned */
>  	u64 search_start;	/* Logical range start we search for csum */
>  	u64 search_len;		/* Logical range len we search for csum */
> +	u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> +				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
>  	unsigned int extent_type;
>  	unsigned int is_hole;
>  	int compressed = 0;
> @@ -1440,6 +1442,32 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
>  				root->objectid, fkey->objectid, fkey->offset);
>  			err |= FILE_EXTENT_ERROR;
>  		}
> +		if (compressed) {
> +			if (extent_num_bytes > root->fs_info->sectorsize) {
> +				error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent ram size, have %llu, max: %u",
> +					root->objectid, fkey->objectid,
> +					fkey->offset, extent_num_bytes,
> +					root->fs_info->sectorsize);
nit: shouldn't this really be sectorsize - 1 ?
> +				err |= FILE_EXTENT_ERROR;
> +			}
> +			if (item_inline_len > max_inline_extent_size) {
> +				error(
> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent on-disk size, have %u, max: %u",
> +					root->objectid, fkey->objectid,
> +					fkey->offset, item_inline_len,
> +					max_inline_extent_size);
> +				err |= FILE_EXTENT_ERROR;
> +			}
> +		} else {
> +			if (extent_num_bytes > max_inline_extent_size) {
> + 			error(
> + "root %llu EXTENT_DATA[%llu %llu] too large inline extent size, have %llu, max: %u",
> + 				root->objectid, fkey->objectid, fkey->offset,
> + 				extent_num_bytes, max_inline_extent_size);
> +				err |= FILE_EXTENT_ERROR;
> +			}
> +		}
>  		if (!compressed && extent_num_bytes != item_inline_len) {
>  			error(
>  		"root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected: %u",
> 

Re: [PATCH v2 4/7] btrfs-progs: check/original mode: Check inline extent size

[Nikolay Borisov]

On 13.03.2018 03:56, Qu Wenruo wrote:
> For inline compressed file extent, kernel doesn't allow inline extent
> ram size larger than sector size and on-disk inline extent size should
> not exceed BTRFS_MAX_INLINE_DATA_SIZE().
> 
> For inline uncompressed file extent, kernel doesn't allow inline extent
> ram and on-disk size larger than either BTRFS_MAX_INLINE_DATA_SIZE() or
> sector size.
> 
> Check it in original mode.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  check/main.c          | 16 ++++++++++++++++
>  check/mode-original.h |  1 +
>  2 files changed, 17 insertions(+)
> 
> diff --git a/check/main.c b/check/main.c
> index 97baae583f04..7821faa929a3 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -560,6 +560,8 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
>  		fprintf(stderr, ", bad file extent");
>  	if (errors & I_ERR_FILE_EXTENT_OVERLAP)
>  		fprintf(stderr, ", file extent overlap");
> +	if (errors & I_ERR_FILE_EXTENT_TOO_LARGE)
> +		fprintf(stderr, ", inline file extent too large");

offtopic:

So in lowmem mode when the error is printed the user is informed what is
the maximum expected size, but in normal mode this is not case? Why is
that? I can see other error messages also don't print out expected values?

>  	if (errors & I_ERR_FILE_EXTENT_DISCOUNT)
>  		fprintf(stderr, ", file extent discount");
>  	if (errors & I_ERR_DIR_ISIZE_WRONG)
> @@ -1433,6 +1435,8 @@ static int process_file_extent(struct btrfs_root *root,
>  	u64 disk_bytenr = 0;
>  	u64 extent_offset = 0;
>  	u64 mask = root->fs_info->sectorsize - 1;
> +	u32 max_inline_size = min_t(u32, mask,
> +				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
>  	int extent_type;
>  	int ret;
>  
> @@ -1458,9 +1462,21 @@ static int process_file_extent(struct btrfs_root *root,
>  	extent_type = btrfs_file_extent_type(eb, fi);
>  
>  	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
> +		u8 compression = btrfs_file_extent_compression(eb, fi);
> +		struct btrfs_item *item = btrfs_item_nr(slot);
> +
>  		num_bytes = btrfs_file_extent_inline_len(eb, slot, fi);
>  		if (num_bytes == 0)
>  			rec->errors |= I_ERR_BAD_FILE_EXTENT;
> +		if (compression) {
> +			if (btrfs_file_extent_inline_item_len(eb, item) >
> +			    max_inline_size ||
> +			    num_bytes > root->fs_info->sectorsize)
> +				rec->errors |= I_ERR_FILE_EXTENT_TOO_LARGE;
> +		} else {
> +			if (num_bytes > max_inline_size)
> +				rec->errors |= I_ERR_FILE_EXTENT_TOO_LARGE;
> +		}
>  		rec->found_size += num_bytes;
>  		num_bytes = (num_bytes + mask) & ~mask;
>  	} else if (extent_type == BTRFS_FILE_EXTENT_REG ||
> diff --git a/check/mode-original.h b/check/mode-original.h
> index f859af478f0f..368de692fdd1 100644
> --- a/check/mode-original.h
> +++ b/check/mode-original.h
> @@ -185,6 +185,7 @@ struct file_extent_hole {
>  #define I_ERR_SOME_CSUM_MISSING		(1 << 12)
>  #define I_ERR_LINK_COUNT_WRONG		(1 << 13)
>  #define I_ERR_FILE_EXTENT_ORPHAN	(1 << 14)
> +#define I_ERR_FILE_EXTENT_TOO_LARGE	(1 << 15)
>  
>  struct inode_record {
>  	struct list_head backrefs;
> 

Re: [PATCH v2 5/7] btrfs-progs: check/lowmem mode: Check inline extent size

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 2670 bytes --]



On 2018年03月13日 16:39, Nikolay Borisov wrote:
> 
> 
> On 13.03.2018 03:56, Qu Wenruo wrote:
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  check/mode-lowmem.c | 28 ++++++++++++++++++++++++++++
>>  1 file changed, 28 insertions(+)
>>
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index 62bcf3d2e126..0d19b373c7af 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -1417,6 +1417,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
>>  	u64 csum_found;		/* In byte size, sectorsize aligned */
>>  	u64 search_start;	/* Logical range start we search for csum */
>>  	u64 search_len;		/* Logical range len we search for csum */
>> +	u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
>> +				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
>>  	unsigned int extent_type;
>>  	unsigned int is_hole;
>>  	int compressed = 0;
>> @@ -1440,6 +1442,32 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
>>  				root->objectid, fkey->objectid, fkey->offset);
>>  			err |= FILE_EXTENT_ERROR;
>>  		}
>> +		if (compressed) {
>> +			if (extent_num_bytes > root->fs_info->sectorsize) {
>> +				error(
>> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent ram size, have %llu, max: %u",
>> +					root->objectid, fkey->objectid,
>> +					fkey->offset, extent_num_bytes,
>> +					root->fs_info->sectorsize);
> nit: shouldn't this really be sectorsize - 1 ?

Yep.

I'd update the github branch.

Thanks,
Qu

>> +				err |= FILE_EXTENT_ERROR;
>> +			}
>> +			if (item_inline_len > max_inline_extent_size) {
>> +				error(
>> +"root %llu EXTENT_DATA[%llu %llu] too large inline extent on-disk size, have %u, max: %u",
>> +					root->objectid, fkey->objectid,
>> +					fkey->offset, item_inline_len,
>> +					max_inline_extent_size);
>> +				err |= FILE_EXTENT_ERROR;
>> +			}
>> +		} else {
>> +			if (extent_num_bytes > max_inline_extent_size) {
>> + 			error(
>> + "root %llu EXTENT_DATA[%llu %llu] too large inline extent size, have %llu, max: %u",
>> + 				root->objectid, fkey->objectid, fkey->offset,
>> + 				extent_num_bytes, max_inline_extent_size);
>> +				err |= FILE_EXTENT_ERROR;
>> +			}
>> +		}
>>  		if (!compressed && extent_num_bytes != item_inline_len) {
>>  			error(
>>  		"root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected: %u",
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]

Re: [PATCH v2 4/7] btrfs-progs: check/original mode: Check inline extent size

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 3986 bytes --]



On 2018年03月13日 16:40, Nikolay Borisov wrote:
> 
> 
> On 13.03.2018 03:56, Qu Wenruo wrote:
>> For inline compressed file extent, kernel doesn't allow inline extent
>> ram size larger than sector size and on-disk inline extent size should
>> not exceed BTRFS_MAX_INLINE_DATA_SIZE().
>>
>> For inline uncompressed file extent, kernel doesn't allow inline extent
>> ram and on-disk size larger than either BTRFS_MAX_INLINE_DATA_SIZE() or
>> sector size.
>>
>> Check it in original mode.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  check/main.c          | 16 ++++++++++++++++
>>  check/mode-original.h |  1 +
>>  2 files changed, 17 insertions(+)
>>
>> diff --git a/check/main.c b/check/main.c
>> index 97baae583f04..7821faa929a3 100644
>> --- a/check/main.c
>> +++ b/check/main.c
>> @@ -560,6 +560,8 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
>>  		fprintf(stderr, ", bad file extent");
>>  	if (errors & I_ERR_FILE_EXTENT_OVERLAP)
>>  		fprintf(stderr, ", file extent overlap");
>> +	if (errors & I_ERR_FILE_EXTENT_TOO_LARGE)
>> +		fprintf(stderr, ", inline file extent too large");
> 
> offtopic:
> 
> So in lowmem mode when the error is printed the user is informed what is
> the maximum expected size, but in normal mode this is not case? Why is
> that? I can see other error messages also don't print out expected values?

The difference is in how different mode handles error.

For normal mode, we use bitmap to indicate error, which can only tell us
what's wrong, but without extra info.

In lowmem mode, we just output the error when we found it, so we have
extra info.

I'm not sure the difference makes sense or not, but at least lowmem mode
is a little more friendly for developer. (For end user, I don't know if
such detailed output would help)


Thanks,
Qu

> 
>>  	if (errors & I_ERR_FILE_EXTENT_DISCOUNT)
>>  		fprintf(stderr, ", file extent discount");
>>  	if (errors & I_ERR_DIR_ISIZE_WRONG)
>> @@ -1433,6 +1435,8 @@ static int process_file_extent(struct btrfs_root *root,
>>  	u64 disk_bytenr = 0;
>>  	u64 extent_offset = 0;
>>  	u64 mask = root->fs_info->sectorsize - 1;
>> +	u32 max_inline_size = min_t(u32, mask,
>> +				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
>>  	int extent_type;
>>  	int ret;
>>  
>> @@ -1458,9 +1462,21 @@ static int process_file_extent(struct btrfs_root *root,
>>  	extent_type = btrfs_file_extent_type(eb, fi);
>>  
>>  	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
>> +		u8 compression = btrfs_file_extent_compression(eb, fi);
>> +		struct btrfs_item *item = btrfs_item_nr(slot);
>> +
>>  		num_bytes = btrfs_file_extent_inline_len(eb, slot, fi);
>>  		if (num_bytes == 0)
>>  			rec->errors |= I_ERR_BAD_FILE_EXTENT;
>> +		if (compression) {
>> +			if (btrfs_file_extent_inline_item_len(eb, item) >
>> +			    max_inline_size ||
>> +			    num_bytes > root->fs_info->sectorsize)
>> +				rec->errors |= I_ERR_FILE_EXTENT_TOO_LARGE;
>> +		} else {
>> +			if (num_bytes > max_inline_size)
>> +				rec->errors |= I_ERR_FILE_EXTENT_TOO_LARGE;
>> +		}
>>  		rec->found_size += num_bytes;
>>  		num_bytes = (num_bytes + mask) & ~mask;
>>  	} else if (extent_type == BTRFS_FILE_EXTENT_REG ||
>> diff --git a/check/mode-original.h b/check/mode-original.h
>> index f859af478f0f..368de692fdd1 100644
>> --- a/check/mode-original.h
>> +++ b/check/mode-original.h
>> @@ -185,6 +185,7 @@ struct file_extent_hole {
>>  #define I_ERR_SOME_CSUM_MISSING		(1 << 12)
>>  #define I_ERR_LINK_COUNT_WRONG		(1 << 13)
>>  #define I_ERR_FILE_EXTENT_ORPHAN	(1 << 14)
>> +#define I_ERR_FILE_EXTENT_TOO_LARGE	(1 << 15)
>>  
>>  struct inode_record {
>>  	struct list_head backrefs;
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]

Re: [PATCH v2 1/7] btrfs-progs: convert/ext2: Fix inline extent creation check

[David Sterba]

On Tue, Mar 13, 2018 at 09:56:10AM +0800, Qu Wenruo wrote:
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Please squash patches 1 and 2 together and write a changelog, somethiing
like is in the cover letter. This is a bugfix, we need to document what
was broken and how it's fixed.

Re: [PATCH v2 7/7] btrfs-progs: test/mkfs: Add test case for rootdir inline extent size

[David Sterba]

On Tue, Mar 13, 2018 at 09:56:16AM +0800, Qu Wenruo wrote:
> Add a test case for mkfs --rootdir, using files with different file
> sizes to check if invalid large inline extent could exist.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  tests/mkfs-tests/014-rootdir-inline-extent/test.sh | 81 ++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100755 tests/mkfs-tests/014-rootdir-inline-extent/test.sh
> 
> diff --git a/tests/mkfs-tests/014-rootdir-inline-extent/test.sh b/tests/mkfs-tests/014-rootdir-inline-extent/test.sh
> new file mode 100755
> index 000000000000..e0765b0bc4e2
> --- /dev/null
> +++ b/tests/mkfs-tests/014-rootdir-inline-extent/test.sh
> @@ -0,0 +1,81 @@
> +#!/bin/bash
> +# Regression test for mkfs.btrfs --rootdir with inline file extents
> +# For any large inline file extent, btrfs check could already report it
> +
> +source "$TOP/tests/common"

          "$TEST_TOP/common"

> +
> +check_global_prereq fallocate
> +check_prereq mkfs.btrfs
> +
> +prepare_test_dev
> +
> +tmp=$(mktemp -d --tmpdir btrfs-progs-mkfs.rootdirXXXXXXX)
> +
> +pagesize=$(getconf PAGESIZE)
> +create_file()
> +{
> +	local size=$1
> +	# Reuse size and filename
> +	run_check fallocate -l $size "$tmp/$size"
> +}
> +
> +test_mkfs_rootdir()
> +{
> +	nodesize=$1
> +	run_check "$TOP/mkfs.btrfs" --nodesize $nodesize -f --rootdir "$tmp" \
> +		"$TEST_DEV"
> +	run_check $SUDO_HELPER "$TOP/btrfs" check "$TEST_DEV"
> +}
> +
> +# File sizes is designed to cross differnet node size, so even
> +# the sectorsize is not 4K, we can still test it well.
> +create_file 512
> +create_file 1024
> +create_file 2048
> +
> +create_file 3994
> +create_file 3995	# For 4K node size, max inline would be 4k - 101
> +create_file 3996
> +
> +create_file 4095
> +create_file 4096
> +create_file 4097
> +
> +create_file 8090
> +create_file 8091
> +create_file 8092
> +
> +create_file 8191
> +create_file 8192
> +create_file 8193
> +
> +create_file 16282
> +create_file 16283
> +create_file 16284
> +
> +create_file 16383
> +create_file 16384
> +create_file 16385
> +
> +create_file 32666
> +create_file 32667
> +create_file 32668
> +
> +create_file 32767
> +create_file 32768
> +create_file 32769
> +
> +create_file 65434
> +create_file 65435
> +create_file 65436
> +
> +create_file 65535
> +create_file 65536
> +create_file 65537

Please rewrite that as a for cycle that adds the -1 and +1 values.

> +
> +for nodesize in 4096 8192 16384 32768 65536; do
> +	if [ $nodesize -ge $pagesize ]; then
> +		test_mkfs_rootdir $nodesize
> +	fi
> +done
> +rm -rf -- "$tmp"
> -- 
> 2.16.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html