2012/12/5, OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>:
> Namjae Jeon <linkinjeon@xxxxxxxxx> writes:
>
>>>> Let me think, if ‘subtree’ checking is enabled then we should check
>>>> the length condition over here also? Please share if there are any
>>>> other comments also.
>>>
>>> I'm not sure what did you mean. Where is "subtree" check you are
>>> talking? This is fh_to_dentry(), so we don't use parent at all, so
>>> length == 3 is enough?
>> With fileID type="FILEID_FAT_WITHOUT_PARENT", fhlen should be '3'
>> With fileId type="FILEID_FAT_WITH_PARENT", fhlen should be '5'
>>
>> While encoding, WITH_PARENT is selected when subtree check is enabled
>> on the NFS Server.
>>
>> So, when decoding request is arrived- fileid type will be among the '2'
>> cases:
>> Now, in case of fh_to_dentry() - when we consider, that the reqquest
>> for fileid type WITH_PARENT
>> then i think the conditions in fh_to_dentry should be:
>>
>> if((fh_type == FILEID_FAT_WITH_PARENT) && fh_len != 5)
>> return NULL;
>> else if (fh_len != 3)
>> return NULL;
>>
>> So, we took care of these '2' conditions within the switch statement
>> based on the 'fh_type'. We can just change the comparision condition
>> from '<' to '!=':
>> switch (fh_type) {
>> + case FILEID_FAT_WITHOUT_PARENT:
>> + if (fh_len != FAT_FID_SIZE_WITHOUT_PARENT)
>> + return NULL;
>> + case FILEID_FAT_WITH_PARENT:
>> + if ((fh_len != FAT_FID_SIZE_WITH_PARENT) &&
>> + (fh_type == FILEID_FAT_WITH_PARENT))
>> + return NULL;
>> + i_pos = fid->i_pos_hi;
>> + i_pos = (i_pos << 32) | (fid->i_pos_low);
>> + inode = __fat_nfs_get_inode(sb, 0, fid->i_gen, i_pos);
>> + break;
>> + }
>>
>> I think there is no need to push the comparision statements in the
>> begining similar to generic_fh_to_dentry.
>
> 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.
Thanks.
> --
> OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
