btrfs-progs: Detect compressed extent without csum

Patches can be fetch from github:
https://github.com/adam900710/btrfs-progs/tree/compress_nodatasum

It's based on v4.16 stable branch.

James Harvey from mail list reports a strange kernel panic, whichs show obviously kernel memory corruption, while after btrfs decompression failure.

It turns out that, some compressed extent get corrupted on-disk, while the inode has NODATASUM set, there is no csum to prevent corrupted mirror being used.

Although the root cause should be buggy lzo implementation, it still shows that btrfs is not following the behavior defined in btrfs(5):

Note
If nodatacow or nodatasum are enabled, compression is disabled.

So at least make btrfs check to detect such problem.

Qu Wenruo (3):
btrfs-progs: check/lowmem: Add checks for compressed extent without
csum
btrfs-progs: check/original: Add checks for compressed extent without
csum
btrfs-progs: fsck-tests: Add test case for detecting compressed extent
without csum

 check/main.c                                  |  10 ++++++--
 check/mode-lowmem.c                           |  18 +++++++++++++
 .../compressed_extent_without_csum.raw.xz     | Bin 0 -> 21996 bytes
 .../032-compressed-nodatasum/test.sh          |  24 ++++++++++++++++++
 4 files changed, 50 insertions(+), 2 deletions(-)
 create mode 100644 tests/fsck-tests/032-compressed-nodatasum/compressed_extent_without_csum.raw.xz
 create mode 100755 tests/fsck-tests/032-compressed-nodatasum/test.sh

2.17.0

There is one report of compressed extent happens in btrfs, but has no csum and then leads to possible decompress error screwing up kernel memory.

Although it's a kernel bug, and won't cause problem until compressed data get corrupted, let's catch such problem in advance.

This patch will catch any unexpected compressed extent with:

1) 0 or less than expected csum

2) nodatasum flag set in the inode item

This is for lowmem mode.

Issue: #134

---
 check/mode-lowmem.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

index dac3201b7d99..8e6d5e8de12a 100644
@@ -1543,6 +1543,24 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
 			      csum_found);
 		}
 	}
+	/*
+	 * Extra check for compressed extents.
+	 * Btrfs doesn't allow NODATASUM and compressed extent co-exist, thus
+	 * all compressed extent should have csum.
+	 */
+	if (compressed && csum_found < search_len) {
+		error(
+"root %llu EXTENT_DATA[%llu %llu] compressed extent must have csum, but only %llu bytes has csum, expect %llu",
+		      root->objectid, fkey->objectid, fkey->offset, csum_found,
+		      search_len);
+		err |= CSUM_ITEM_MISSING;
+	}
+	if (compressed && nodatasum) {
+		error(
+"root %llu EXTENT_DATA[%llu %llu] is compressed, but inode flag doesn't allow it",
+		      root->objectid, fkey->objectid, fkey->offset);
+		err |= FILE_EXTENT_ERROR;
+	}
 
 	/* Check EXTENT_DATA hole */
 	if (!no_holes && *end != fkey->offset) {

2.17.0

There is one report of compressed extent happens in btrfs, but has no csum and then leads to possible decompress error screwing up kernel memory.

Although it's a kernel bug, and won't cause problem until compressed data get corrupted, let's catch such problem in advance.

This patch will catch any unexpected compressed extent with:

1) 0 or less than expected csum

2) nodatasum flag set in the inode item

This is for original mode.

Issue: #134

---
 check/main.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

index 891a6d797756..db064b27e84b 100644
@@ -1437,6 +1437,7 @@ static int process_file_extent(struct btrfs_root *root,
 	u64 mask = root->fs_info->sectorsize - 1;
 	u32 max_inline_size = min_t(u32, mask,
 				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
+	u8 compression;
 	int extent_type;
 	int ret;
 

@@ -1460,9 +1461,9 @@ static int process_file_extent(struct btrfs_root *root,
 
 	fi = btrfs_item_ptr(eb, slot, struct btrfs_file_extent_item);
 	extent_type = btrfs_file_extent_type(eb, fi);
+	compression = btrfs_file_extent_compression(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);

@@ -1494,6 +1495,8 @@ static int process_file_extent(struct btrfs_root *root,
 		     btrfs_file_extent_encryption(eb, fi) ||
 		     btrfs_file_extent_other_encoding(eb, fi)))
 			rec->errors |= I_ERR_BAD_FILE_EXTENT;
+		if (compression && rec->nodatasum)
+			rec->errors |= I_ERR_BAD_FILE_EXTENT;
 		if (disk_bytenr > 0)
 			rec->found_size += num_bytes;
 	} else {

@@ -1512,7 +1515,8 @@ static int process_file_extent(struct btrfs_root *root,
 	if (disk_bytenr > 0 &&
 	    btrfs_header_owner(eb) != BTRFS_DATA_RELOC_TREE_OBJECTID) {
 		u64 found;
-		if (btrfs_file_extent_compression(eb, fi))
+
+		if (compression)
 			num_bytes = btrfs_file_extent_disk_num_bytes(eb, fi);
 		else
 			disk_bytenr += extent_offset;

@@ -1526,6 +1530,8 @@ static int process_file_extent(struct btrfs_root *root,
 				rec->found_csum_item = 1;
 			if (found < num_bytes)
 				rec->some_csum_missing = 1;
+			if (compression && found < num_bytes)
+				rec->errors |= I_ERR_SOME_CSUM_MISSING;
 		} else if (extent_type == BTRFS_FILE_EXTENT_PREALLOC) {
 			if (found > 0) {
 				ret = check_prealloc_extent_written(root->fs_info,

2.17.0

---
 .../compressed_extent_without_csum.raw.xz     | Bin 0 -> 21996 bytes
 .../032-compressed-nodatasum/test.sh          |  24 ++++++++++++++++++
 2 files changed, 24 insertions(+)
 create mode 100644 tests/fsck-tests/032-compressed-nodatasum/compressed_extent_without_csum.raw.xz
 create mode 100755 tests/fsck-tests/032-compressed-nodatasum/test.sh

new file mode 100644
index 0000000000000000000000000000000000000000..72a3bfd8f8b0bf2d610ce84af0abf4003204a9a1
GIT binary patch

new file mode 100755
index 000000000000..ebec74dffdbd
@@ -0,0 +1,24 @@
+#!/bin/bash
+#
+# Verify that btrfs check can detect compressed extent without csum as an error
+#
+# There is report about nodatasum inode which has compressed extent, and when
+# its compressed data is corrupted, decompress screw up the whole kernel.
+#
+# While btrfs(5) shows that nodatasum will disable data CoW and compression,
+# kernel obviously doesn't follow it well. And in above case, lzo problem
+# can leads to more serious kernel memory corruption since btrfs completely
+# depends its csum to prevent corruption.
+#
+# So btrfs check should report such compressed extent without csum as error.
+
+source "$TEST_TOP/common"
+
+check_prereq btrfs
+
+check_image() {
+	run_mustfail "btrfs fails to detect compressed extent with csum as an error" \
+		"$TOP/btrfs" check "$1"
+}
+
+check_all_images

2.17.0

Sorry for late reply, can you please refresh the patches on top of a more recent progs? Thanks.

https://github.com/adam900710/btrfs-progs/tree/compress_nodatasum

Branch updated.

Rebased to v4.17.1 tag, with first patch commit message change:
"1) 0 or less than expected csum" -> "1) missing csum"

And re-numbering the test case of the last patch from 032 to 036.

Thanks,
Qu