On 2019/3/20 下午2:37, Qu Wenruo wrote:
[snip]
> +
> + /*
> + * Since btrfs device add doesn't check device size at all, we could
> + * have device item whose size is smaller than 1M which is useless, but
> + * still valid.
> + * So here we can only check the obviously wrong case.
> + */
> + if (btrfs_device_total_bytes(leaf, ditem) == 0) {
> + dev_item_err(fs_info, leaf, slot,
> + "invalid total bytes: have 0");
> + goto error;
> + }
Hi David,
Please remove this patch from misc-next queue.
Under the following call trace, we could create device with total_bytes
== 0;
btrfs_rm_device()
|- btrfs_shrink_device()
| |- btrfs_device_set_total_bytes(device, 0)
| |- btrfs_update_device()
| |- btrfs_commit_transaction() #1
|- btrfs_rm_dev_item()
This will trigger write time tree checker warning.
And further more, this can create valid btrfs with device->total_bytes
== 0 and triggering read time tree-checker if power loss happens after
above transaction #1 but before next transaction.
So this dev item check is too restrict.
And furthermore, the error output is misleading, its devid is extracted
from key->objectid, but it should be key->offset.
For the fuzzed image, I'd like to fix it by either enhancing the seed
device lookup procedure.
Thanks,
Qu
> + if (btrfs_device_bytes_used(leaf, ditem) >
> + btrfs_device_total_bytes(leaf, ditem)) {
> + dev_item_err(fs_info, leaf, slot,
> + "invalid bytes used: have %llu expect [0, %llu]",
> + btrfs_device_bytes_used(leaf, ditem),
> + btrfs_device_total_bytes(leaf, ditem));
> + goto error;
> + }
> + /*
> + * Remaining members like io_align/type/gen/dev_group aren't really
> + * utilized.
> + * Skip them to make later usage of them easier.
> + */
> + return 0;
> +error:
> + return -EUCLEAN;
> +}
> +
> /*
> * Common point to switch the item-specific validation.
> */
> @@ -632,6 +712,9 @@ static int check_leaf_item(struct btrfs_fs_info *fs_info,
> ret = btrfs_check_chunk_valid(fs_info, leaf, chunk,
> key->offset);
> break;
> + case BTRFS_DEV_ITEM_KEY:
> + ret = check_dev_item(fs_info, leaf, key, slot);
> + break;
> }
> return ret;
> }
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 645ffc9c47b0..7510272408e8 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -4958,15 +4958,6 @@ static void check_raid56_incompat_flag(struct
> btrfs_fs_info *info, u64 type)
> btrfs_set_fs_incompat(info, RAID56);
> }
>
> -#define BTRFS_MAX_DEVS(info) ((BTRFS_MAX_ITEM_SIZE(info) \
> - - sizeof(struct btrfs_chunk)) \
> - / sizeof(struct btrfs_stripe) + 1)
> -
> -#define BTRFS_MAX_DEVS_SYS_CHUNK ((BTRFS_SYSTEM_CHUNK_ARRAY_SIZE \
> - - 2 * sizeof(struct btrfs_disk_key) \
> - - 2 * sizeof(struct btrfs_chunk)) \
> - / sizeof(struct btrfs_stripe) + 1)
> -
> static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
> u64 start, u64 type)
> {
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index 3ad9d58d1b66..38ed94b77202 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -258,6 +258,15 @@ struct btrfs_fs_devices {
>
> #define BTRFS_BIO_INLINE_CSUM_SIZE 64
>
> +#define BTRFS_MAX_DEVS(info) ((BTRFS_MAX_ITEM_SIZE(info) \
> + - sizeof(struct btrfs_chunk)) \
> + / sizeof(struct btrfs_stripe) + 1)
> +
> +#define BTRFS_MAX_DEVS_SYS_CHUNK ((BTRFS_SYSTEM_CHUNK_ARRAY_SIZE \
> + - 2 * sizeof(struct btrfs_disk_key) \
> + - 2 * sizeof(struct btrfs_chunk)) \
> + / sizeof(struct btrfs_stripe) + 1)
> +
> /*
> * we need the mirror number and stripe index to be passed around
> * the call chain while we are processing end_io (especially errors).
>
