Re: [PATCH v5 5/8] fat: restructure export_operations

[Namjae Jeon <linkinjeon@gmail.com>]

2012/12/5, OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>:
> Namjae Jeon <linkinjeon@gmail.com> writes:
>
>>> I can understand what is doing. I'm asking why there is difference.
>>>
>>> 1) generic_fh_to_dentry() allows (*_PARENT && fh_len == 2).
>>> 2) fat_fh_to_dentry_nostale() doesn't allows (*_PARENT && fh_len == 3).
>>>
>>> Why does logic has difference?
>>
>> When we consider the generic routine of encode_fh() and the structure
>> ‘fid’
>>
>> struct fid {
>>       struct {
>>             u32 ino;
>>             u32 gen;
>>             u32 parent_ino;
>>             u32 parent_gen;
>>       } i32;
>> };
>>
>> fh_len= 2(without parent)
>> fh_len=4(with parent)
>>
>> Condition checking in export_encode_fh()
>> {
>>
>>    if (parent && (len < 4)) {
>>                 *max_len = 4;
>>                 return FILEID_INVALID;
>>         } else if (len < 2) {
>>                 *max_len = 2;
>>                 return FILEID_INVALID;
>>         }
>>         ...
>>                 len = 2;
>>         ...
>>                 if (parent) {
>> ..
>>                                 len = 4;
>>                                 type = FILEID_INO32_GEN_PARENT;
>>                 }
>> …
>> }
>>
>> The logic does take care of altering the length for the ‘2’ cases
>> with/without parent.
>> So, while encoding -> the care has been taken for length checking but
>> while decoding(generic_fh_to_dentry) the length check is not put in
>> place.
>> I think it should be done in the generic routine also.
>>
>> It should be:
>> if ((fh_len != 2 && fh_len != 4) ||
>>                 (fh_type != FILEID_INO32_GEN && fh_type !=
>> FILEID_INO32_GEN_PARENT))
>>     return NULL;
>>
>> Please share your opinion.
>
> I know encode_fh(). But NFS is network protocol, and network can input
> any data, and I guess the userland interface (open_by_handle()?) can be
> any too.
>
> And generic_fh_to_dentry()'s input verify choose to check the minimum
> length only. But your logic choose the exact length.
>
> I think the both is sane and correct. But I wonder why did you changed it.
There was no particular reason for us to put those conditions. It is
just we knew what fh lengths we have chosen for the 2 cases
WITH/WITHOUT parent.
i.e., we checked with encoded length.
Now, when I check the export functions of other filesystems(btrfs,
nilfs2, udf). They also adopt the same method of checking the exact
length and type.
If there is any particular reason, we will look into that and can also
updated on that.

Thanks.
> --
> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
>