On 2018年01月19日 17:40, Nikolay Borisov wrote: > > > On 19.01.2018 09:25, Qu Wenruo wrote: >> btrfs_match_dir_item_name() will check if its filetype is valid before >> doing search, this makes btrfs-progs unable to locate and remove invalid >> dir_index for btrfs_unlink().> >> This function only affects btrfs_link() and btrfs_unlink() in upper >> layer, and normal check can find invalid filetype by itself. > > There is no function btrfs_link in btrfs-progs, there is, however, > btrfs_add_link did you mean that function?
Yep, sorry for the wrong name.
> However, it doesn't seem to
> use verify_dir_item hence the check you are removing. I think this part
> of the commit log should be more precise. Also it's not really clear
> what you mean by "normal check" in this sentence.
I skipped several function calls.
btrfs_add_link()
|- check_dir_conflict
|- btrfs_lookup_dir_index() / btrfs_lookup_dir_item()
|- btrfs_match_dir_item_name()
|- verify_dir_item()
And for btrfs_add_link() without checking invalid filetype, we can avoid
case like inserting duplicated DIR_ITEM/DIR_INDEX to avoid further
screwing up things.
For btrfs_unlink() it will be different story, as btrfs_unlink() can
locate the conflicting but invalid DIR_ITEM/DIR_INDEX and remove it,
allow us to fix the problem.
And by "normal check" I mean btrfs check.
I'll add the call trace to it.
>>
>> So remove the filetype check is completely safe in this case, and will
>> enhance btrfs_unlink() to remove invalid dir_index/dir_item for repair.
>
> So the problem is that since btrfs_unlink calls verify_item and the
> latter has the filetype check in case of wrong filetype (corruption)
> verify_dir_item fails, hence we cannot perform the unlink? If my
> understanding is correct how about something like:
>
>
> If we have a corrupted dir item and enough information to repair it we
> need to first delete the old/corrupted version and then insert a new
> one.
> However, btrfs_unlink calls btrfs_match_dir_item_name to locate the
> offending dir item for deletion. The latter, in turn, uses
> verify_dir_item which checks if the value for DIR item's type is sane.
> In case of a corrupted type value then verify_dir_item will fail which
> in turn will prevent btrfs_unlink from deleting the offending item.
Your understanding is completely right, and much shorter than my planned
explanation.
I'll take it with extra call trace.
Thanks,
Qu
>
>>
>> Signed-off-by: Qu Wenruo <w...@suse.com>
>> ---
>> dir-item.c | 6 ------
>> 1 file changed, 6 deletions(-)
>>
>> diff --git a/dir-item.c b/dir-item.c
>> index 462546c0eaf4..e0a0ab4d7a5d 100644
>> --- a/dir-item.c
>> +++ b/dir-item.c
>> @@ -294,12 +294,6 @@ static int verify_dir_item(struct btrfs_root *root,
>> u16 namelen = BTRFS_NAME_LEN;
>> u8 type = btrfs_dir_type(leaf, dir_item);
>>
>> - if (type >= BTRFS_FT_MAX) {
>> - fprintf(stderr, "invalid dir item type: %d\n",
>> - (int)type);
>> - return 1;
>> - }
>> -
>> if (type == BTRFS_FT_XATTR)
>> namelen = XATTR_NAME_MAX;
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
signature.asc
Description: OpenPGP digital signature
