Re: [PATCH] f2fs: add additional sanity check in f2fs_acl_from_disk()

[Chao Yu <yuchao0@huawei.com>]

On 2018/8/31 0:19, cgxu519 wrote:
> 
> On 08/30/2018 11:41 PM, Chao Yu wrote:
>> Hi Chengguang,
>>
>> On 2018/8/30 21:33, Chengguang Xu wrote:
>>> Add additinal sanity check for irregular case(e.g. corruption).
>>> If size of extended attribution is smaller than size of acl header,
>>> then return -EINVAL.
>>>
>>> Signed-off-by: Chengguang Xu <cgxu519@gmx.com>
>>> ---
>>>   fs/f2fs/acl.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
>>> index 111824199a88..79e9ea773070 100644
>>> --- a/fs/f2fs/acl.c
>>> +++ b/fs/f2fs/acl.c
>>> @@ -53,6 +53,9 @@ static struct posix_acl *f2fs_acl_from_disk(const char *value, size_t size)
>>>   	struct f2fs_acl_entry *entry = (struct f2fs_acl_entry *)(hdr + 1);
>>>   	const char *end = value + size;
>>>   
>>> +	if (size < sizeof(f2fs_acl_header))
>>> +		return ERR_PTR(-EINVAL);
>> I guess below codes have checked that already?
>>
>> 	count = f2fs_acl_count(size);
>> 	if (count < 0)
>> 		return ERR_PTR(-EINVAL);
> 
> Hi Chao,
> 
> Thanks for prompt reply.
> 
> I still think in a rare case, it can pass the check in f2fs_acl_count() 
> and cause unexpected behavior.
> 
> For example, like below code path in f2fs_acl_count().

if size < sizeof(f2fs_acl_header)

size -= sizeof(struct f2fs_acl_header);

size should be smaller than zero, right?

> 
> -> if (s < 0) {
>              if (size % sizeof(struct f2fs_acl_entry_short))
>                       return -1;
> ->        return size / sizeof(struct f2fs_acl_entry_short);

So the return value should be smaller than zero?

Thanks,

> }
> 
> 
> Thanks,
> Chengguang
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel