From: Nikolay Borisov <nborisov@suse.com>
To: Qu Wenruo <wqu@suse.com>, linux-btrfs@vger.kernel.org
Cc: Yoon Jungyeon <jungyeon@gatech.edu>
Subject: Re: [PATCH 1/6] btrfs: tree-checker: Verify chunk items
Date: Wed, 13 Mar 2019 11:19:00 +0200 [thread overview]
Message-ID: <8612b9e4-bf67-32db-0c37-504e748354f8@suse.com> (raw)
In-Reply-To: <20190313085511.23540-2-wqu@suse.com>
On 13.03.19 г. 10:55 ч., Qu Wenruo wrote:
> We already have btrfs_check_chunk_valid() to verify each chunk before
> tree-checker.
>
> Merge that function into tree-checker, and update its error message to
> be more readable.
>
> Old error message would be something like:
> BTRFS error (device dm-3): invalid chunk num_stipres: 0
>
> New error message would be:
> Btrfs critical (device dm-3): corrupt superblock syschunk array: chunk_start=2097152, invalid chunk num_stripes: 0
> Or
> Btrfs critical (device dm-3): corrupt leaf: root=3 block=8388608 slot=3 chunk_start=2097152, invalid chunk num_stripes: 0
>
> Btrfs_check_chunk_valid() is exported for super block syschunk array
> verification.
>
> Also make tree-checker to verify chunk items, this makes chunk item
> checker covers all chunks and avoid fuzzed image.
>
> Reported-by: Yoon Jungyeon <jungyeon@gatech.edu>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=202751
> Signed-off-by: Qu Wenruo <wqu@suse.com>
I had already reviewed the earlier posting of this patch so:
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/tree-checker.c | 152 ++++++++++++++++++++++++++++++++++++++++
> fs/btrfs/tree-checker.h | 3 +
> fs/btrfs/volumes.c | 94 +------------------------
> 3 files changed, 156 insertions(+), 93 deletions(-)
>
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index b8cdaf472031..fe3f37c23c29 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -448,6 +448,152 @@ static int check_block_group_item(struct btrfs_fs_info *fs_info,
> return 0;
> }
>
> +__printf(5, 6)
> +__cold
> +static void chunk_err(const struct btrfs_fs_info *fs_info,
> + const struct extent_buffer *leaf,
> + const struct btrfs_chunk *chunk, u64 logical,
> + const char *fmt, ...)
> +{
> + /* Only superblock eb is able to have such small offset */
> + bool is_sb = (leaf->start == BTRFS_SUPER_INFO_OFFSET);
> + struct va_format vaf;
> + va_list args;
> + int i;
> + int slot = -1;
> +
> + if (!is_sb) {
> + /*
> + * Get the slot number by iterating through all slots, this
> + * would provide better readability.
> + */
> + for (i = 0; i < btrfs_header_nritems(leaf); i++) {
> + if (btrfs_item_ptr_offset(leaf, i) ==
> + (unsigned long) chunk) {
> + slot = i;
> + break;
> + }
> + }
> + }
> + va_start(args, fmt);
> + vaf.fmt = fmt;
> + vaf.va = &args;
> +
> + if (is_sb)
> + btrfs_crit(fs_info,
> + "corrupt superblock syschunk array: chunk_start=%llu, %pV",
> + logical, &vaf);
> + else
> + btrfs_crit(fs_info,
> + "corrupt leaf: root=%llu block=%llu slot=%d chunk_start=%llu, %pV",
> + BTRFS_CHUNK_TREE_OBJECTID, leaf->start, slot,
> + logical, &vaf);
> + va_end(args);
> +}
> +
> +/*
> + * The common chunk check which could also work on super block sys chunk array.
> + *
> + * Return -EUCLEAN if anything is corrupted.
> + * Return 0 if everything is OK.
> + */
> +int btrfs_check_chunk_valid(struct btrfs_fs_info *fs_info,
> + struct extent_buffer *leaf,
> + struct btrfs_chunk *chunk, u64 logical)
> +{
> + u64 length;
> + u64 stripe_len;
> + u16 num_stripes;
> + u16 sub_stripes;
> + u64 type;
> + u64 features;
> + bool mixed = false;
> +
> + length = btrfs_chunk_length(leaf, chunk);
> + stripe_len = btrfs_chunk_stripe_len(leaf, chunk);
> + num_stripes = btrfs_chunk_num_stripes(leaf, chunk);
> + sub_stripes = btrfs_chunk_sub_stripes(leaf, chunk);
> + type = btrfs_chunk_type(leaf, chunk);
> +
> + if (!num_stripes) {
> + chunk_err(fs_info, leaf, chunk, logical,
> + "invalid chunk num_stripes: %u", num_stripes);
> + return -EUCLEAN;
> + }
> + if (!IS_ALIGNED(logical, fs_info->sectorsize)) {
> + chunk_err(fs_info, leaf, chunk, logical,
> + "invalid chunk logical %llu", logical);
> + return -EUCLEAN;
> + }
> + if (btrfs_chunk_sector_size(leaf, chunk) != fs_info->sectorsize) {
> + chunk_err(fs_info, leaf, chunk, logical,
> + "invalid chunk sectorsize %u",
> + btrfs_chunk_sector_size(leaf, chunk));
> + return -EUCLEAN;
> + }
> + if (!length || !IS_ALIGNED(length, fs_info->sectorsize)) {
> + chunk_err(fs_info, leaf, chunk, logical,
> + "invalid chunk length %llu", length);
> + return -EUCLEAN;
> + }
> + if (!is_power_of_2(stripe_len) || stripe_len != BTRFS_STRIPE_LEN) {
> + chunk_err(fs_info, leaf, chunk, logical,
> + "invalid chunk stripe length: %llu", stripe_len);
> + return -EUCLEAN;
> + }
> + if (~(BTRFS_BLOCK_GROUP_TYPE_MASK | BTRFS_BLOCK_GROUP_PROFILE_MASK) &
> + type) {
> + chunk_err(fs_info, leaf, chunk, logical,
> + "unrecognized chunk type: %llu",
> + ~(BTRFS_BLOCK_GROUP_TYPE_MASK |
> + BTRFS_BLOCK_GROUP_PROFILE_MASK) &
> + btrfs_chunk_type(leaf, chunk));
> + return -EUCLEAN;
> + }
> +
> + if ((type & BTRFS_BLOCK_GROUP_TYPE_MASK) == 0) {
> + chunk_err(fs_info, leaf, chunk, logical,
> + "missing chunk type flag: 0x%llx", type);
> + return -EUCLEAN;
> + }
> +
> + if ((type & BTRFS_BLOCK_GROUP_SYSTEM) &&
> + (type & (BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_DATA))) {
> + chunk_err(fs_info, leaf, chunk, logical,
> + "system chunk with data or metadata type: 0x%llx", type);
> + return -EUCLEAN;
> + }
> +
> + features = btrfs_super_incompat_flags(fs_info->super_copy);
> + if (features & BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS)
> + mixed = true;
> +
> + if (!mixed) {
> + if ((type & BTRFS_BLOCK_GROUP_METADATA) &&
> + (type & BTRFS_BLOCK_GROUP_DATA)) {
> + chunk_err(fs_info, leaf, chunk, logical,
> + "mixed chunk type in non-mixed mode: 0x%llx", type);
> + return -EUCLEAN;
> + }
> + }
> +
> + if ((type & BTRFS_BLOCK_GROUP_RAID10 && sub_stripes != 2) ||
> + (type & BTRFS_BLOCK_GROUP_RAID1 && num_stripes < 1) ||
> + (type & BTRFS_BLOCK_GROUP_RAID5 && num_stripes < 2) ||
> + (type & BTRFS_BLOCK_GROUP_RAID6 && num_stripes < 3) ||
> + (type & BTRFS_BLOCK_GROUP_DUP && num_stripes > 2) ||
> + ((type & BTRFS_BLOCK_GROUP_PROFILE_MASK) == 0 &&
> + num_stripes != 1)) {
> + chunk_err(fs_info, leaf, chunk, logical,
> + "invalid num_stripes:sub_stripes %u:%u for profile %llu",
> + num_stripes, sub_stripes,
> + type & BTRFS_BLOCK_GROUP_PROFILE_MASK);
> + return -EUCLEAN;
> + }
> +
> + return 0;
> +}
> +
> /*
> * Common point to switch the item-specific validation.
> */
> @@ -455,6 +601,7 @@ static int check_leaf_item(struct btrfs_fs_info *fs_info,
> struct extent_buffer *leaf,
> struct btrfs_key *key, int slot)
> {
> + struct btrfs_chunk *chunk;
> int ret = 0;
>
> switch (key->type) {
> @@ -472,6 +619,11 @@ static int check_leaf_item(struct btrfs_fs_info *fs_info,
> case BTRFS_BLOCK_GROUP_ITEM_KEY:
> ret = check_block_group_item(fs_info, leaf, key, slot);
> break;
> + case BTRFS_CHUNK_ITEM_KEY:
> + chunk = btrfs_item_ptr(leaf, slot, struct btrfs_chunk);
> + ret = btrfs_check_chunk_valid(fs_info, leaf, chunk,
> + key->offset);
> + break;
> }
> return ret;
> }
> diff --git a/fs/btrfs/tree-checker.h b/fs/btrfs/tree-checker.h
> index 6f8d1b627c53..6cb96ba5e711 100644
> --- a/fs/btrfs/tree-checker.h
> +++ b/fs/btrfs/tree-checker.h
> @@ -33,4 +33,7 @@ int btrfs_check_leaf_write(struct btrfs_fs_info *fs_info,
> struct extent_buffer *leaf);
> int btrfs_check_node(struct btrfs_fs_info *fs_info, struct extent_buffer *node);
>
> +int btrfs_check_chunk_valid(struct btrfs_fs_info *fs_info,
> + struct extent_buffer *leaf,
> + struct btrfs_chunk *chunk, u64 logical);
> #endif
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 15561926ab32..0e3822870f1e 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -27,6 +27,7 @@
> #include "math.h"
> #include "dev-replace.h"
> #include "sysfs.h"
> +#include "tree-checker.h"
>
> const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
> [BTRFS_RAID_RAID10] = {
> @@ -6705,99 +6706,6 @@ struct btrfs_device *btrfs_alloc_device(struct btrfs_fs_info *fs_info,
> return dev;
> }
>
> -/* Return -EIO if any error, otherwise return 0. */
> -static int btrfs_check_chunk_valid(struct btrfs_fs_info *fs_info,
> - struct extent_buffer *leaf,
> - struct btrfs_chunk *chunk, u64 logical)
> -{
> - u64 length;
> - u64 stripe_len;
> - u16 num_stripes;
> - u16 sub_stripes;
> - u64 type;
> - u64 features;
> - bool mixed = false;
> -
> - length = btrfs_chunk_length(leaf, chunk);
> - stripe_len = btrfs_chunk_stripe_len(leaf, chunk);
> - num_stripes = btrfs_chunk_num_stripes(leaf, chunk);
> - sub_stripes = btrfs_chunk_sub_stripes(leaf, chunk);
> - type = btrfs_chunk_type(leaf, chunk);
> -
> - if (!num_stripes) {
> - btrfs_err(fs_info, "invalid chunk num_stripes: %u",
> - num_stripes);
> - return -EIO;
> - }
> - if (!IS_ALIGNED(logical, fs_info->sectorsize)) {
> - btrfs_err(fs_info, "invalid chunk logical %llu", logical);
> - return -EIO;
> - }
> - if (btrfs_chunk_sector_size(leaf, chunk) != fs_info->sectorsize) {
> - btrfs_err(fs_info, "invalid chunk sectorsize %u",
> - btrfs_chunk_sector_size(leaf, chunk));
> - return -EIO;
> - }
> - if (!length || !IS_ALIGNED(length, fs_info->sectorsize)) {
> - btrfs_err(fs_info, "invalid chunk length %llu", length);
> - return -EIO;
> - }
> - if (!is_power_of_2(stripe_len) || stripe_len != BTRFS_STRIPE_LEN) {
> - btrfs_err(fs_info, "invalid chunk stripe length: %llu",
> - stripe_len);
> - return -EIO;
> - }
> - if (~(BTRFS_BLOCK_GROUP_TYPE_MASK | BTRFS_BLOCK_GROUP_PROFILE_MASK) &
> - type) {
> - btrfs_err(fs_info, "unrecognized chunk type: %llu",
> - ~(BTRFS_BLOCK_GROUP_TYPE_MASK |
> - BTRFS_BLOCK_GROUP_PROFILE_MASK) &
> - btrfs_chunk_type(leaf, chunk));
> - return -EIO;
> - }
> -
> - if ((type & BTRFS_BLOCK_GROUP_TYPE_MASK) == 0) {
> - btrfs_err(fs_info, "missing chunk type flag: 0x%llx", type);
> - return -EIO;
> - }
> -
> - if ((type & BTRFS_BLOCK_GROUP_SYSTEM) &&
> - (type & (BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_DATA))) {
> - btrfs_err(fs_info,
> - "system chunk with data or metadata type: 0x%llx", type);
> - return -EIO;
> - }
> -
> - features = btrfs_super_incompat_flags(fs_info->super_copy);
> - if (features & BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS)
> - mixed = true;
> -
> - if (!mixed) {
> - if ((type & BTRFS_BLOCK_GROUP_METADATA) &&
> - (type & BTRFS_BLOCK_GROUP_DATA)) {
> - btrfs_err(fs_info,
> - "mixed chunk type in non-mixed mode: 0x%llx", type);
> - return -EIO;
> - }
> - }
> -
> - if ((type & BTRFS_BLOCK_GROUP_RAID10 && sub_stripes != 2) ||
> - (type & BTRFS_BLOCK_GROUP_RAID1 && num_stripes < 1) ||
> - (type & BTRFS_BLOCK_GROUP_RAID5 && num_stripes < 2) ||
> - (type & BTRFS_BLOCK_GROUP_RAID6 && num_stripes < 3) ||
> - (type & BTRFS_BLOCK_GROUP_DUP && num_stripes > 2) ||
> - ((type & BTRFS_BLOCK_GROUP_PROFILE_MASK) == 0 &&
> - num_stripes != 1)) {
> - btrfs_err(fs_info,
> - "invalid num_stripes:sub_stripes %u:%u for profile %llu",
> - num_stripes, sub_stripes,
> - type & BTRFS_BLOCK_GROUP_PROFILE_MASK);
> - return -EIO;
> - }
> -
> - return 0;
> -}
> -
> static void btrfs_report_missing_device(struct btrfs_fs_info *fs_info,
> u64 devid, u8 *uuid, bool error)
> {
>
next prev parent reply other threads:[~2019-03-13 9:19 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-13 8:55 [PATCH 0/6] btrfs: Enhance tree checker and runtime checker to handle the new wave of fuzzed image attack Qu Wenruo
2019-03-13 8:55 ` [PATCH 1/6] btrfs: tree-checker: Verify chunk items Qu Wenruo
2019-03-13 9:19 ` Nikolay Borisov [this message]
2019-03-19 14:50 ` David Sterba
2019-03-20 0:46 ` Qu Wenruo
2019-03-20 5:03 ` Qu Wenruo
2019-03-13 8:55 ` [PATCH 2/6] btrfs: tree-checker: Verify dev item Qu Wenruo
2019-03-13 9:19 ` Nikolay Borisov
2019-03-13 8:55 ` [PATCH 3/6] btrfs: Check the first key and level for cached extent buffer Qu Wenruo
2019-03-13 9:24 ` Nikolay Borisov
2019-03-13 8:55 ` [PATCH 4/6] btrfs: tree-checker: Enhance chunk checker to validate chunk profiler Qu Wenruo
2019-03-13 9:18 ` Nikolay Borisov
2019-03-13 8:55 ` [PATCH 5/6] btrfs: tree-checker: Verify inode item Qu Wenruo
2019-03-13 9:28 ` Nikolay Borisov
2019-03-13 8:55 ` [PATCH 6/6] btrfs: inode: Verify inode mode to avoid NULL pointer dereference Qu Wenruo
2019-03-13 9:41 ` Nikolay Borisov
2019-03-13 9:01 ` [PATCH 0/6] btrfs: Enhance tree checker and runtime checker to handle the new wave of fuzzed image attack Qu Wenruo
2019-03-19 15:34 ` David Sterba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8612b9e4-bf67-32db-0c37-504e748354f8@suse.com \
--to=nborisov@suse.com \
--cc=jungyeon@gatech.edu \
--cc=linux-btrfs@vger.kernel.org \
--cc=wqu@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).