Re: [PATCH] btrfs: fix false alert caused by legacy btrfs root item

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

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



On 2020/9/22 下午6:20, Martin Steigerwald wrote:
> Hi Qu.
> 
> Instead of the tool, can I also patch my kernel with the patch below to 
> have it automatically fix it?

Sure, this one is a little safer than the tool.

> 
> If so, which approach would you prefer for testing?
> 
> I can apply the patch as I compile kernels myself.

That's great.

That should solve the problem.

And if you don't like the legacy root item, just do a balance (no matter
data or metadata), and that legacy root item will be converted to
current one, and even affected kernel won't report any error any more.

Thanks,
Qu

> 
> Thanks,
> Martin
> 
> Qu Wenruo - 22.09.20, 04:37:01 CEST:
>> Commit 259ee7754b67 ("btrfs: tree-checker: Add ROOT_ITEM check")
>> introduced btrfs root item size check, however btrfs root item has two
>> format, the legacy one which just ends before generation_v2 member,
>> is smaller than current btrfs root item size.
>>
>> This caused btrfs kernel to reject valid but old tree root leaves.
>>
>> Fix this problem by also allowing legacy root item, since kernel can
>> already handle them pretty well and upgrade to newer root item format
>> when needed.
>>
>> Reported-by: Martin Steigerwald <martin@lichtvoll.de>
>> Fixes: 259ee7754b67 ("btrfs: tree-checker: Add ROOT_ITEM check")
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  fs/btrfs/tree-checker.c         | 17 ++++++++++++-----
>>  include/uapi/linux/btrfs_tree.h |  9 +++++++++
>>  2 files changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
>> index 7b1fee630f97..6f794aca48d3 100644
>> --- a/fs/btrfs/tree-checker.c
>> +++ b/fs/btrfs/tree-checker.c
>> @@ -1035,7 +1035,7 @@ static int check_root_item(struct extent_buffer
>> *leaf, struct btrfs_key *key, int slot)
>>  {
>>  	struct btrfs_fs_info *fs_info = leaf->fs_info;
>> -	struct btrfs_root_item ri;
>> +	struct btrfs_root_item ri = { 0 };
>>  	const u64 valid_root_flags = BTRFS_ROOT_SUBVOL_RDONLY |
>>  				     BTRFS_ROOT_SUBVOL_DEAD;
>>  	int ret;
>> @@ -1044,14 +1044,21 @@ static int check_root_item(struct
>> extent_buffer *leaf, struct btrfs_key *key, if (ret < 0)
>>  		return ret;
>>
>> -	if (btrfs_item_size_nr(leaf, slot) != sizeof(ri)) {
>> +	if (btrfs_item_size_nr(leaf, slot) != sizeof(ri) &&
>> +	    btrfs_item_size_nr(leaf, slot) != 
> btrfs_legacy_root_item_size())
>> { generic_err(leaf, slot,
>> -			    "invalid root item size, have %u expect %zu",
>> -			    btrfs_item_size_nr(leaf, slot), sizeof(ri));
>> +			    "invalid root item size, have %u expect %zu or 
> %zu",
>> +			    btrfs_item_size_nr(leaf, slot), sizeof(ri),
>> +			    btrfs_legacy_root_item_size());
>>  	}
>>
>> +	/*
>> +	 * For legacy root item, the members starting at generation_v2 
> will
>> be +	 * all filled with 0.
>> +	 * And since we allow geneartion_v2 as 0, it will still pass the
>> check. +	 */
>>  	read_extent_buffer(leaf, &ri, btrfs_item_ptr_offset(leaf, slot),
>> -			   sizeof(ri));
>> +			   btrfs_item_size_nr(leaf, slot));
>>
>>  	/* Generation related */
>>  	if (btrfs_root_generation(&ri) >
>> diff --git a/include/uapi/linux/btrfs_tree.h
>> b/include/uapi/linux/btrfs_tree.h index 9ba64ca6b4ac..464095a28b18
>> 100644
>> --- a/include/uapi/linux/btrfs_tree.h
>> +++ b/include/uapi/linux/btrfs_tree.h
>> @@ -644,6 +644,15 @@ struct btrfs_root_item {
>>  	__le64 reserved[8]; /* for future */
>>  } __attribute__ ((__packed__));
>>
>> +/*
>> + * Btrfs root item used to be smaller than current size.
>> + * The old format ends at where member generation_v2 is.
>> + */
>> +static inline size_t btrfs_legacy_root_item_size(void)
>> +{
>> +	return offsetof(struct btrfs_root_item, generation_v2);
>> +}
>> +
>>  /*
>>   * this is used for both forward and backward root refs
>>   */
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]