On Wed, Mar 13, 2019 at 04:55:06PM +0800, 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 <[email protected]>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=202751
> Signed-off-by: Qu Wenruo <[email protected]>
> ---
> fs/btrfs/tree-checker.c | 152 ++++++++++++++++++++++++++++++++++++++++
> fs/btrfs/tree-checker.h | 3 +
> fs/btrfs/volumes.c | 94 +------------------------
> 3 files changed, 156 insertions(+), 93 deletions(-)
Please split the patch to part where you just move the code and where
the logic is changed. btrfs_check_chunk_valid is not the same, old has
-EIO and new -EUCLEAN. Moving a function is ok in the same patch if
there's no change.
>
> 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);
Please move the initialization and comment out of the declaration block
> + 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;
> + }
> + }
> + }
