linux-kernel - Re: [PATCH v5 5/8] fat: restructure export

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 6 Dec 2012 16:37:41 +0900
From:	Namjae Jeon <linkinjeon@...il.com>
To:	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
Cc:	akpm@...ux-foundation.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Namjae Jeon <namjae.jeon@...sung.com>,
	Ravishankar N <ravi.n1@...sung.com>,
	Amit Sahrawat <a.sahrawat@...sung.com>
Subject: Re: [PATCH v5 5/8] fat: restructure export_operations

2012/12/5, OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>:
> Namjae Jeon <linkinjeon@...il.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@...l.parknet.co.jp>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/