Skip to contentSkip to conversation

btrfs-progs: Don't BUG_ON() if we failed to load one device or one chunk

PATCH Applied Qu Wenruo 3 participants 4 messages Jul 3, 2018 – Jul 4, 2018 Btrfs file system

lore
Share & tools
lore

Qu Wenruo

author
#0 lore raw

PATCH 1/1 btrfs-progs: Don't BUG_ON() if we failed to load one device or one chunk

Don't panic for btrfs_read_chunk_tree() if one device or chunk is corrupted.
Caller can already handle it pretty well.

Link https://bugzilla.kernel.org/show_bug.cgi?id=199839
Signed-off-by Qu Wenruo <[email protected]>
---
 volumes.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
volumes.c +4 -2
index 9379d2f61eff..24eb3e8b2578 100644
@@ -2158,13 +2158,15 @@ int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info)
 			dev_item = btrfs_item_ptr(leaf, slot,
 						  struct btrfs_dev_item);
 			ret = read_one_dev(fs_info, leaf, dev_item);
-			BUG_ON(ret);
+			if (ret < 0)
+				goto error;
 		} else if (found_key.type == BTRFS_CHUNK_ITEM_KEY) {
 			struct btrfs_chunk *chunk;
 			chunk = btrfs_item_ptr(leaf, slot, struct btrfs_chunk);
 			ret = read_one_chunk(fs_info, &found_key, leaf, chunk,
 					     slot);
-			BUG_ON(ret);
+			if (ret < 0)
+				goto error;
 		}
 		path->slots[0]++;
 	}
Signature · 1 line
2.18.0
#2 lore raw
Quoted 5 lines · David Sterba
Quoted 4 lines · Qu Wenruo

Don't panic for btrfs_read_chunk_tree() if one device or chunk is corrupted.
Caller can already handle it pretty well.

Quoted 7 lines
Quoted 2 lines
Link https://bugzilla.kernel.org/show_bug.cgi?id=199839
Signed-off-by Qu Wenruo <[email protected]>

Applied, thanks.

In case there's a fuzzed image, please send it too. There are example commits how to name, format and document it. Thanks.

Sorry for forgetting the image.

However, I'm puzzled by the fact that, even for some fuzzed image which is originally designed to detect kernel misbehavior, it's still added into btrfs-progs test cases.

Shouldn't such kernel oriented case locate some place else?

Thanks,
Qu

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
Quoted 13 lines · Qu Wenruo
Quoted 10 lines · David Sterba
Quoted 6 lines · Qu Wenruo

Don't panic for btrfs_read_chunk_tree() if one device or chunk is corrupted.
Caller can already handle it pretty well.

Link https://bugzilla.kernel.org/show_bug.cgi?id=199839
Signed-off-by Qu Wenruo <[email protected]>

Applied, thanks.

Quoted 8 lines
Quoted 2 lines

In case there's a fuzzed image, please send it too. There are example commits how to name, format and document it. Thanks.

Sorry for forgetting the image.

However, I'm puzzled by the fact that, even for some fuzzed image which is originally designed to detect kernel misbehavior, it's still added into btrfs-progs test cases.

The kernel tests of the fuzzed images are considered dangerous and the mount test is intentionally missing from the progs for now. As the testsuite can be exported and used outside of the git repository, the actual mount tests can be run in an environment where the crashes don't matter. We'd need to add the script and some way to skip it by default.

The fuzzed images should also pass the userspace tools, for better testing coverage eg. for the error handling so they're kept in one place.