diff mbox

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

Message ID 20180703134040.8108-1-wqu@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Qu Wenruo July 3, 2018, 1:40 p.m. UTC
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(-)

Comments

David Sterba July 4, 2018, 11:58 a.m. UTC | #1
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.
--
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
Qu Wenruo July 4, 2018, 12:01 p.m. UTC | #2
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
>
David Sterba July 4, 2018, 12:17 p.m. UTC | #3
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.
--
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
diff mbox

Patch

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]++;
 	}