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

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

[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 <wqu@suse.com>
---
 volumes.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/volumes.c b/volumes.c
index 9379d2f61eff..24eb3e8b2578 100644
--- a/volumes.c
+++ b/volumes.c
@@ -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]++;
 	}
-- 
2.18.0

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

[David Sterba]

On Tue, Jul 03, 2018 at 09:40:40PM +0800, Qu Wenruo wrote:
> 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 <wqu@suse.com>

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.

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

[Qu Wenruo]

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



On 2018年07月04日 19:58, David Sterba wrote:
> On Tue, Jul 03, 2018 at 09:40:40PM +0800, Qu Wenruo wrote:
>> 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 <wqu@suse.com>
> 
> 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

> --
> 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: 488 bytes --]

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

[David Sterba]

On Wed, Jul 04, 2018 at 08:01:11PM +0800, Qu Wenruo wrote:
> 
> 
> On 2018年07月04日 19:58, David Sterba wrote:
> > On Tue, Jul 03, 2018 at 09:40:40PM +0800, Qu Wenruo wrote:
> >> 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 <wqu@suse.com>
> > 
> > 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.

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.