All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Fix chunk num_stripes FPE error found by fuzzed image
@ 2016-08-29  8:08 Qu Wenruo
  2016-08-29  8:08 ` [PATCH 1/4] btrfs-progs: Enhance and export print_objectid function Qu Wenruo
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Qu Wenruo @ 2016-08-29  8:08 UTC (permalink / raw)
  To: linux-btrfs

This patchset will fix chunk num_stripes FPE error, by introducing another 2
layers of check.

1) Check key type validation against leaf owner(for original mode)
   This will prevent invalid key, like CHUNK_ITEM key in root tree, to be
   passed to later check.

   For original mode, it uses leaf's owner as standard.
   For low memory mode, it will use root->objectid(while not in devel
   branch yet)

   This is an extra early check layer, which could benefit other part.

2) Check chunk item again before processing it
   This is the root fix, as final defense for chunk item.
   This check itself can already fix the problem.
   But 1) can give more info on which is really going wrong in the fs.

Thanks for Lukas for his fuzzed images and report.

Qu Wenruo (4):
  btrfs-progs: Enhance and export print_objectid function
  btrfs-progs: Enhance and export print_key_type function
  btrfs-progs: Ignore invalid key in invalid root
  btrfs-progs: Do extra chunk check before processing chunk item

 cmds-check.c |  78 ++++++++++++++++++++++++++++++++
 print-tree.c | 145 ++++++++++++++++++++++++++++++-----------------------------
 print-tree.h |   2 +
 volumes.c    |   8 ++--
 volumes.h    |   4 ++
 5 files changed, 161 insertions(+), 76 deletions(-)

-- 
2.9.3




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] btrfs-progs: Enhance and export print_objectid function
  2016-08-29  8:08 [PATCH 0/4] Fix chunk num_stripes FPE error found by fuzzed image Qu Wenruo
@ 2016-08-29  8:08 ` Qu Wenruo
  2016-08-29  8:09 ` [PATCH 2/4] btrfs-progs: Enhance and export print_key_type function Qu Wenruo
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2016-08-29  8:08 UTC (permalink / raw)
  To: linux-btrfs

This function is quite useful for a lot of error report.
Enhance it to support custom output other than stdout.
And export it for later btrfsck enhancement.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 print-tree.c | 59 ++++++++++++++++++++++++++++++-----------------------------
 print-tree.h |  1 +
 2 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/print-tree.c b/print-tree.c
index 81ab81f..601fc0f 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -707,97 +707,98 @@ static void print_key_type(u64 objectid, u8 type)
 	};
 }
 
-static void print_objectid(u64 objectid, u8 type)
+void print_objectid(FILE *stream, u64 objectid, u8 type)
 {
 	switch (type) {
 	case BTRFS_DEV_EXTENT_KEY:
-		printf("%llu", (unsigned long long)objectid); /* device id */
+		/* device id */
+		fprintf(stream, "%llu", (unsigned long long)objectid);
 		return;
 	case BTRFS_QGROUP_RELATION_KEY:
-		printf("%llu/%llu", btrfs_qgroup_level(objectid),
+		fprintf(stream, "%llu/%llu", btrfs_qgroup_level(objectid),
 		       btrfs_qgroup_subvid(objectid));
 		return;
 	case BTRFS_UUID_KEY_SUBVOL:
 	case BTRFS_UUID_KEY_RECEIVED_SUBVOL:
-		printf("0x%016llx", (unsigned long long)objectid);
+		fprintf(stream, "0x%016llx", (unsigned long long)objectid);
 		return;
 	}
 
 	switch (objectid) {
 	case BTRFS_ROOT_TREE_OBJECTID:
 		if (type == BTRFS_DEV_ITEM_KEY)
-			printf("DEV_ITEMS");
+			fprintf(stream, "DEV_ITEMS");
 		else
-			printf("ROOT_TREE");
+			fprintf(stream, "ROOT_TREE");
 		break;
 	case BTRFS_EXTENT_TREE_OBJECTID:
-		printf("EXTENT_TREE");
+		fprintf(stream, "EXTENT_TREE");
 		break;
 	case BTRFS_CHUNK_TREE_OBJECTID:
-		printf("CHUNK_TREE");
+		fprintf(stream, "CHUNK_TREE");
 		break;
 	case BTRFS_DEV_TREE_OBJECTID:
-		printf("DEV_TREE");
+		fprintf(stream, "DEV_TREE");
 		break;
 	case BTRFS_FS_TREE_OBJECTID:
-		printf("FS_TREE");
+		fprintf(stream, "FS_TREE");
 		break;
 	case BTRFS_ROOT_TREE_DIR_OBJECTID:
-		printf("ROOT_TREE_DIR");
+		fprintf(stream, "ROOT_TREE_DIR");
 		break;
 	case BTRFS_CSUM_TREE_OBJECTID:
-		printf("CSUM_TREE");
+		fprintf(stream, "CSUM_TREE");
 		break;
 	case BTRFS_BALANCE_OBJECTID:
-		printf("BALANCE");
+		fprintf(stream, "BALANCE");
 		break;
 	case BTRFS_ORPHAN_OBJECTID:
-		printf("ORPHAN");
+		fprintf(stream, "ORPHAN");
 		break;
 	case BTRFS_TREE_LOG_OBJECTID:
-		printf("TREE_LOG");
+		fprintf(stream, "TREE_LOG");
 		break;
 	case BTRFS_TREE_LOG_FIXUP_OBJECTID:
-		printf("LOG_FIXUP");
+		fprintf(stream, "LOG_FIXUP");
 		break;
 	case BTRFS_TREE_RELOC_OBJECTID:
-		printf("TREE_RELOC");
+		fprintf(stream, "TREE_RELOC");
 		break;
 	case BTRFS_DATA_RELOC_TREE_OBJECTID:
-		printf("DATA_RELOC_TREE");
+		fprintf(stream, "DATA_RELOC_TREE");
 		break;
 	case BTRFS_EXTENT_CSUM_OBJECTID:
-		printf("EXTENT_CSUM");
+		fprintf(stream, "EXTENT_CSUM");
 		break;
 	case BTRFS_FREE_SPACE_OBJECTID:
-		printf("FREE_SPACE");
+		fprintf(stream, "FREE_SPACE");
 		break;
 	case BTRFS_FREE_INO_OBJECTID:
-		printf("FREE_INO");
+		fprintf(stream, "FREE_INO");
 		break;
 	case BTRFS_QUOTA_TREE_OBJECTID:
-		printf("QUOTA_TREE");
+		fprintf(stream, "QUOTA_TREE");
 		break;
 	case BTRFS_UUID_TREE_OBJECTID:
-		printf("UUID_TREE");
+		fprintf(stream, "UUID_TREE");
 		break;
 	case BTRFS_FREE_SPACE_TREE_OBJECTID:
-		printf("FREE_SPACE_TREE");
+		fprintf(stream, "FREE_SPACE_TREE");
 		break;
 	case BTRFS_MULTIPLE_OBJECTIDS:
-		printf("MULTIPLE");
+		fprintf(stream, "MULTIPLE");
 		break;
 	case (u64)-1:
-		printf("-1");
+		fprintf(stream, "-1");
 		break;
 	case BTRFS_FIRST_CHUNK_TREE_OBJECTID:
 		if (type == BTRFS_CHUNK_ITEM_KEY) {
-			printf("FIRST_CHUNK_TREE");
+			fprintf(stream, "FIRST_CHUNK_TREE");
 			break;
 		}
 		/* fall-thru */
 	default:
-		printf("%llu", (unsigned long long)objectid);
+		fprintf(stream, "%llu", (unsigned long long)objectid);
 	}
 }
 
@@ -808,7 +809,7 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key)
 	u64 offset = btrfs_disk_key_offset(disk_key);
 
 	printf("key (");
-	print_objectid(objectid, type);
+	print_objectid(stdout, objectid, type);
 	printf(" ");
 	print_key_type(objectid, type);
 	switch (type) {
diff --git a/print-tree.h b/print-tree.h
index f0153c1..73fb3e8 100644
--- a/print-tree.h
+++ b/print-tree.h
@@ -24,4 +24,5 @@ void btrfs_print_tree(struct btrfs_root *root, struct extent_buffer *t, int foll
 void btrfs_print_key(struct btrfs_disk_key *disk_key);
 void print_chunk(struct extent_buffer *eb, struct btrfs_chunk *chunk);
 void print_extent_item(struct extent_buffer *eb, int slot, int metadata);
+void print_objectid(FILE *stream, u64 objectid, u8 type);
 #endif
-- 
2.9.3




^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] btrfs-progs: Enhance and export print_key_type function
  2016-08-29  8:08 [PATCH 0/4] Fix chunk num_stripes FPE error found by fuzzed image Qu Wenruo
  2016-08-29  8:08 ` [PATCH 1/4] btrfs-progs: Enhance and export print_objectid function Qu Wenruo
@ 2016-08-29  8:09 ` Qu Wenruo
  2016-08-29  8:09 ` [PATCH 3/4] btrfs-progs: Ignore invalid key in invalid root Qu Wenruo
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2016-08-29  8:09 UTC (permalink / raw)
  To: linux-btrfs

Just the same thing done for print_objectid().

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 print-tree.c | 86 ++++++++++++++++++++++++++++++------------------------------
 print-tree.h |  1 +
 2 files changed, 44 insertions(+), 43 deletions(-)

diff --git a/print-tree.c b/print-tree.c
index 601fc0f..f97482f 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -577,133 +577,133 @@ static void print_free_space_header(struct extent_buffer *leaf, int slot)
 	       (unsigned long long)btrfs_free_space_bitmaps(leaf, header));
 }
 
-static void print_key_type(u64 objectid, u8 type)
+void print_key_type(FILE *stream, u64 objectid, u8 type)
 {
 	if (type == 0 && objectid == BTRFS_FREE_SPACE_OBJECTID) {
-		printf("UNTYPED");
+		fprintf(stream, "UNTYPED");
 		return;
 	}
 
 	switch (type) {
 	case BTRFS_INODE_ITEM_KEY:
-		printf("INODE_ITEM");
+		fprintf(stream, "INODE_ITEM");
 		break;
 	case BTRFS_INODE_REF_KEY:
-		printf("INODE_REF");
+		fprintf(stream, "INODE_REF");
 		break;
 	case BTRFS_INODE_EXTREF_KEY:
-		printf("INODE_EXTREF");
+		fprintf(stream, "INODE_EXTREF");
 		break;
 	case BTRFS_DIR_ITEM_KEY:
-		printf("DIR_ITEM");
+		fprintf(stream, "DIR_ITEM");
 		break;
 	case BTRFS_DIR_INDEX_KEY:
-		printf("DIR_INDEX");
+		fprintf(stream, "DIR_INDEX");
 		break;
 	case BTRFS_DIR_LOG_ITEM_KEY:
-		printf("DIR_LOG_ITEM");
+		fprintf(stream, "DIR_LOG_ITEM");
 		break;
 	case BTRFS_DIR_LOG_INDEX_KEY:
-		printf("DIR_LOG_INDEX");
+		fprintf(stream, "DIR_LOG_INDEX");
 		break;
 	case BTRFS_XATTR_ITEM_KEY:
-		printf("XATTR_ITEM");
+		fprintf(stream, "XATTR_ITEM");
 		break;
 	case BTRFS_ORPHAN_ITEM_KEY:
-		printf("ORPHAN_ITEM");
+		fprintf(stream, "ORPHAN_ITEM");
 		break;
 	case BTRFS_ROOT_ITEM_KEY:
-		printf("ROOT_ITEM");
+		fprintf(stream, "ROOT_ITEM");
 		break;
 	case BTRFS_ROOT_REF_KEY:
-		printf("ROOT_REF");
+		fprintf(stream, "ROOT_REF");
 		break;
 	case BTRFS_ROOT_BACKREF_KEY:
-		printf("ROOT_BACKREF");
+		fprintf(stream, "ROOT_BACKREF");
 		break;
 	case BTRFS_EXTENT_ITEM_KEY:
-		printf("EXTENT_ITEM");
+		fprintf(stream, "EXTENT_ITEM");
 		break;
 	case BTRFS_METADATA_ITEM_KEY:
-		printf("METADATA_ITEM");
+		fprintf(stream, "METADATA_ITEM");
 		break;
 	case BTRFS_TREE_BLOCK_REF_KEY:
-		printf("TREE_BLOCK_REF");
+		fprintf(stream, "TREE_BLOCK_REF");
 		break;
 	case BTRFS_SHARED_BLOCK_REF_KEY:
-		printf("SHARED_BLOCK_REF");
+		fprintf(stream, "SHARED_BLOCK_REF");
 		break;
 	case BTRFS_EXTENT_DATA_REF_KEY:
-		printf("EXTENT_DATA_REF");
+		fprintf(stream, "EXTENT_DATA_REF");
 		break;
 	case BTRFS_SHARED_DATA_REF_KEY:
-		printf("SHARED_DATA_REF");
+		fprintf(stream, "SHARED_DATA_REF");
 		break;
 	case BTRFS_EXTENT_REF_V0_KEY:
-		printf("EXTENT_REF_V0");
+		fprintf(stream, "EXTENT_REF_V0");
 		break;
 	case BTRFS_CSUM_ITEM_KEY:
-		printf("CSUM_ITEM");
+		fprintf(stream, "CSUM_ITEM");
 		break;
 	case BTRFS_EXTENT_CSUM_KEY:
-		printf("EXTENT_CSUM");
+		fprintf(stream, "EXTENT_CSUM");
 		break;
 	case BTRFS_EXTENT_DATA_KEY:
-		printf("EXTENT_DATA");
+		fprintf(stream, "EXTENT_DATA");
 		break;
 	case BTRFS_BLOCK_GROUP_ITEM_KEY:
-		printf("BLOCK_GROUP_ITEM");
+		fprintf(stream, "BLOCK_GROUP_ITEM");
 		break;
 	case BTRFS_FREE_SPACE_INFO_KEY:
-		printf("FREE_SPACE_INFO");
+		fprintf(stream, "FREE_SPACE_INFO");
 		break;
 	case BTRFS_FREE_SPACE_EXTENT_KEY:
-		printf("FREE_SPACE_EXTENT");
+		fprintf(stream, "FREE_SPACE_EXTENT");
 		break;
 	case BTRFS_FREE_SPACE_BITMAP_KEY:
-		printf("FREE_SPACE_BITMAP");
+		fprintf(stream, "FREE_SPACE_BITMAP");
 		break;
 	case BTRFS_CHUNK_ITEM_KEY:
-		printf("CHUNK_ITEM");
+		fprintf(stream, "CHUNK_ITEM");
 		break;
 	case BTRFS_DEV_ITEM_KEY:
-		printf("DEV_ITEM");
+		fprintf(stream, "DEV_ITEM");
 		break;
 	case BTRFS_DEV_EXTENT_KEY:
-		printf("DEV_EXTENT");
+		fprintf(stream, "DEV_EXTENT");
 		break;
 	case BTRFS_BALANCE_ITEM_KEY:
-		printf("BALANCE_ITEM");
+		fprintf(stream, "BALANCE_ITEM");
 		break;
 	case BTRFS_DEV_REPLACE_KEY:
-		printf("DEV_REPLACE");
+		fprintf(stream, "DEV_REPLACE");
 		break;
 	case BTRFS_STRING_ITEM_KEY:
-		printf("STRING_ITEM");
+		fprintf(stream, "STRING_ITEM");
 		break;
 	case BTRFS_QGROUP_STATUS_KEY:
-		printf("QGROUP_STATUS");
+		fprintf(stream, "QGROUP_STATUS");
 		break;
 	case BTRFS_QGROUP_RELATION_KEY:
-		printf("QGROUP_RELATION");
+		fprintf(stream, "QGROUP_RELATION");
 		break;
 	case BTRFS_QGROUP_INFO_KEY:
-		printf("QGROUP_INFO");
+		fprintf(stream, "QGROUP_INFO");
 		break;
 	case BTRFS_QGROUP_LIMIT_KEY:
-		printf("QGROUP_LIMIT");
+		fprintf(stream, "QGROUP_LIMIT");
 		break;
 	case BTRFS_DEV_STATS_KEY:
-		printf("DEV_STATS");
+		fprintf(stream, "DEV_STATS");
 		break;
 	case BTRFS_UUID_KEY_SUBVOL:
-		printf("UUID_KEY_SUBVOL");
+		fprintf(stream, "UUID_KEY_SUBVOL");
 		break;
 	case BTRFS_UUID_KEY_RECEIVED_SUBVOL:
-		printf("UUID_KEY_RECEIVED_SUBVOL");
+		fprintf(stream, "UUID_KEY_RECEIVED_SUBVOL");
 		break;
 	default:
-		printf("UNKNOWN.%d", type);
+		fprintf(stream, "UNKNOWN.%d", type);
 	};
 }
 
@@ -811,7 +811,7 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key)
 	printf("key (");
 	print_objectid(stdout, objectid, type);
 	printf(" ");
-	print_key_type(objectid, type);
+	print_key_type(stdout, objectid, type);
 	switch (type) {
 	case BTRFS_QGROUP_RELATION_KEY:
 	case BTRFS_QGROUP_INFO_KEY:
diff --git a/print-tree.h b/print-tree.h
index 73fb3e8..9865500 100644
--- a/print-tree.h
+++ b/print-tree.h
@@ -25,4 +25,5 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key);
 void print_chunk(struct extent_buffer *eb, struct btrfs_chunk *chunk);
 void print_extent_item(struct extent_buffer *eb, int slot, int metadata);
 void print_objectid(FILE *stream, u64 objectid, u8 type);
+void print_key_type(FILE *stream, u64 objectid, u8 type);
 #endif
-- 
2.9.3




^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] btrfs-progs: Ignore invalid key in invalid root
  2016-08-29  8:08 [PATCH 0/4] Fix chunk num_stripes FPE error found by fuzzed image Qu Wenruo
  2016-08-29  8:08 ` [PATCH 1/4] btrfs-progs: Enhance and export print_objectid function Qu Wenruo
  2016-08-29  8:09 ` [PATCH 2/4] btrfs-progs: Enhance and export print_key_type function Qu Wenruo
@ 2016-08-29  8:09 ` Qu Wenruo
  2016-08-29  8:09 ` [PATCH 4/4] btrfs-progs: Do extra chunk check before processing chunk item Qu Wenruo
  2016-08-29 16:28 ` [PATCH 0/4] Fix chunk num_stripes FPE error found by fuzzed image David Sterba
  4 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2016-08-29  8:09 UTC (permalink / raw)
  To: linux-btrfs

Btrfs tree implies a lot of restriction on which key types are allowed
in specific roots.

Like CHUNK_ITEM keys are only valid in chunk root.

This patch will add such check at run_next_block() for original mode.

Reported-by: Lukas Lueg <lukas.lueg@gmail.com>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 cmds-check.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/cmds-check.c b/cmds-check.c
index 0ddfd24..617b867 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -6122,6 +6122,58 @@ full_backref:
 	return 0;
 }
 
+static void report_mismatch_key_root(u8 key_type, u64 rootid)
+{
+	fprintf(stderr, "Invalid key type(");
+	print_key_type(stderr, 0, key_type);
+	fprintf(stderr, ") found in root(");
+	print_objectid(stderr, rootid, 0);
+	fprintf(stderr, ")\n");
+}
+
+/*
+ * Check if the key is valid with its extent buffer.
+ *
+ * This is a early check in case invalid key exists in a extent buffer
+ * This is not comprehensive yet, but should prevent wrong key/item passed
+ * further
+ */
+static int check_type_with_root(u64 rootid, u8 key_type)
+{
+	switch (key_type) {
+	/* Only valid in chunk tree */
+	case BTRFS_DEV_ITEM_KEY:
+	case BTRFS_CHUNK_ITEM_KEY:
+		if (rootid != BTRFS_CHUNK_TREE_OBJECTID)
+			goto err;
+		break;
+	/* valid in csum and log tree */
+	case BTRFS_CSUM_TREE_OBJECTID:
+		if (!(rootid == BTRFS_TREE_LOG_OBJECTID ||
+		      is_fstree(rootid)))
+			goto err;
+		break;
+	case BTRFS_EXTENT_ITEM_KEY:
+	case BTRFS_METADATA_ITEM_KEY:
+	case BTRFS_BLOCK_GROUP_ITEM_KEY:
+		if (rootid != BTRFS_EXTENT_TREE_OBJECTID)
+			goto err;
+		break;
+	case BTRFS_ROOT_ITEM_KEY:
+		if (rootid != BTRFS_ROOT_TREE_OBJECTID)
+			goto err;
+		break;
+	case BTRFS_DEV_EXTENT_KEY:
+		if (rootid != BTRFS_DEV_TREE_OBJECTID)
+			goto err;
+		break;
+	}
+	return 0;
+err:
+	report_mismatch_key_root(key_type, rootid);
+	return -EINVAL;
+}
+
 static int run_next_block(struct btrfs_root *root,
 			  struct block_info *bits,
 			  int bits_nr,
@@ -6271,6 +6323,16 @@ static int run_next_block(struct btrfs_root *root,
 		for (i = 0; i < nritems; i++) {
 			struct btrfs_file_extent_item *fi;
 			btrfs_item_key_to_cpu(buf, &key, i);
+			/*
+			 * Check key type against the leaf owner.
+			 * Could filter quite a lot of early error if
+			 * owner is correct
+			 */
+			if (check_type_with_root(btrfs_header_owner(buf),
+						 key.type)) {
+				fprintf(stderr, "ignoring invalid key\n");
+				continue;
+			}
 			if (key.type == BTRFS_EXTENT_ITEM_KEY) {
 				process_extent_item(root, extent_cache, buf,
 						    i);
-- 
2.9.3




^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] btrfs-progs: Do extra chunk check before processing chunk item
  2016-08-29  8:08 [PATCH 0/4] Fix chunk num_stripes FPE error found by fuzzed image Qu Wenruo
                   ` (2 preceding siblings ...)
  2016-08-29  8:09 ` [PATCH 3/4] btrfs-progs: Ignore invalid key in invalid root Qu Wenruo
@ 2016-08-29  8:09 ` Qu Wenruo
  2016-08-29 16:28 ` [PATCH 0/4] Fix chunk num_stripes FPE error found by fuzzed image David Sterba
  4 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2016-08-29  8:09 UTC (permalink / raw)
  To: linux-btrfs

Current we only do chunk validation check at mount time.

It's good for most case, but for fuzzed or manually crafted images, we
can insert a CHUNK_ITEM key into root tree.

Since mount time check will only check chunk tree, it will not check
CHUNK_ITEM in root tree.

Even with previous key type check against leaf owner, it is still
possible to modify the leaf owner to by-pass it.

So we still need to check chunk validation before processing it.

Reported-by: Lukas Lueg <lukas.lueg@gmail.com>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 cmds-check.c | 16 ++++++++++++++++
 volumes.c    |  8 ++++----
 volumes.h    |  4 ++++
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index 617b867..1e1f7c9 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -5220,8 +5220,24 @@ static int process_chunk_item(struct cache_tree *chunk_cache,
 			      int slot)
 {
 	struct chunk_record *rec;
+	struct btrfs_chunk *chunk;
 	int ret = 0;
 
+	chunk = btrfs_item_ptr(eb, slot, struct btrfs_chunk);
+	/*
+	 * Do extra check for this chunk item,
+	 *
+	 * It's still possible one can craft a leaf with CHUNK_ITEM, with
+	 * wrong onwer(3) out of chunk tree, to pass both chunk tree check
+	 * and owner<->key_type check.
+	 */
+	ret = btrfs_check_chunk_valid(global_info->tree_root, eb, chunk, slot,
+				      key->offset);
+	if (ret < 0) {
+		error("chunk(%llu, %llu) is not valid, ignore it",
+		      key->offset, btrfs_chunk_length(eb, chunk));
+		return 0;
+	}
 	rec = btrfs_new_chunk_record(eb, key, slot);
 	ret = insert_cache_extent(chunk_cache, &rec->cache);
 	if (ret) {
diff --git a/volumes.c b/volumes.c
index 9a5580a..2d07e66 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1614,10 +1614,10 @@ static struct btrfs_device *fill_missing_device(u64 devid)
  * slot == -1: SYSTEM chunk
  * return -EIO on error, otherwise return 0
  */
-static int btrfs_check_chunk_valid(struct btrfs_root *root,
-				   struct extent_buffer *leaf,
-				   struct btrfs_chunk *chunk,
-				   int slot, u64 logical)
+int btrfs_check_chunk_valid(struct btrfs_root *root,
+			    struct extent_buffer *leaf,
+			    struct btrfs_chunk *chunk,
+			    int slot, u64 logical)
 {
 	u64 length;
 	u64 stripe_len;
diff --git a/volumes.h b/volumes.h
index af7182b..d7b7d3c 100644
--- a/volumes.h
+++ b/volumes.h
@@ -226,4 +226,8 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
 			     struct extent_buffer *eb,
 			     struct btrfs_multi_bio *multi,
 			     u64 stripe_len, u64 *raid_map);
+int btrfs_check_chunk_valid(struct btrfs_root *root,
+			    struct extent_buffer *leaf,
+			    struct btrfs_chunk *chunk,
+			    int slot, u64 logical);
 #endif
-- 
2.9.3




^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/4] Fix chunk num_stripes FPE error found by fuzzed image
  2016-08-29  8:08 [PATCH 0/4] Fix chunk num_stripes FPE error found by fuzzed image Qu Wenruo
                   ` (3 preceding siblings ...)
  2016-08-29  8:09 ` [PATCH 4/4] btrfs-progs: Do extra chunk check before processing chunk item Qu Wenruo
@ 2016-08-29 16:28 ` David Sterba
  4 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2016-08-29 16:28 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Mon, Aug 29, 2016 at 04:08:58PM +0800, Qu Wenruo wrote:
> This patchset will fix chunk num_stripes FPE error, by introducing another 2
> layers of check.
> 
> 1) Check key type validation against leaf owner(for original mode)
>    This will prevent invalid key, like CHUNK_ITEM key in root tree, to be
>    passed to later check.
> 
>    For original mode, it uses leaf's owner as standard.
>    For low memory mode, it will use root->objectid(while not in devel
>    branch yet)
> 
>    This is an extra early check layer, which could benefit other part.
> 
> 2) Check chunk item again before processing it
>    This is the root fix, as final defense for chunk item.
>    This check itself can already fix the problem.
>    But 1) can give more info on which is really going wrong in the fs.
> 
> Thanks for Lukas for his fuzzed images and report.

Thans for the testing and fixes. Can you please also add the images to
tests/fuzz-tests/images ?


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-08-29 16:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-29  8:08 [PATCH 0/4] Fix chunk num_stripes FPE error found by fuzzed image Qu Wenruo
2016-08-29  8:08 ` [PATCH 1/4] btrfs-progs: Enhance and export print_objectid function Qu Wenruo
2016-08-29  8:09 ` [PATCH 2/4] btrfs-progs: Enhance and export print_key_type function Qu Wenruo
2016-08-29  8:09 ` [PATCH 3/4] btrfs-progs: Ignore invalid key in invalid root Qu Wenruo
2016-08-29  8:09 ` [PATCH 4/4] btrfs-progs: Do extra chunk check before processing chunk item Qu Wenruo
2016-08-29 16:28 ` [PATCH 0/4] Fix chunk num_stripes FPE error found by fuzzed image David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.