Re: [PATCH] fat: Support fallocate on fat.

[Namjae Jeon]

2012/7/9, OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>:
> Namjae Jeon <linkinjeon@xxxxxxxxx> writes:
>
>> 2012/7/9, OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>:
>>> Namjae Jeon <linkinjeon@xxxxxxxxx> writes:
>>>
>>>>>> +	/*
>>>>>> +	 * calculate i_blocks and mmu_private from the actual number of
>>>>>> +	 * allocated clusters instead of doing it from file size.This
>>>>>> ensures
>>>>>> +	 * that the preallocated disk space using FALLOC_FL_KEEP_SIZE is
>>>>>> +	 * persistent across remounts and writes go into the allocated
>>>>>> clusters.
>>>>>> +	 */
>>>>>> +	fat_calc_dir_size(inode);
>>>>>
>>>>> Looks like the wrong. If you didn't initialize preallocated space, the
>>>>> data never be exposed to userland. It is security bug.
>>>> As explained above, if we do append write instead of seeking into a
>>>> random offset, there is no security risk.
>>>
>>> So it means? - if we didn't, there is.
>> Yes, right.
>>>
>>>> The main disadvantage with initializing the preallocated space (as is
>>>> done in case of without FALLOC_FL_KEEP_SIZE ) is it takes long time
>>>> for bigger allocation sizes. It took ~70 seconds to preallocate 2GB on
>>>> our target if FALLOC_FL_KEEP_SIZE is not set.
>>>
>>> It doesn't become the reason to expose uninitialized data.
>> I agree.. If I try to change code for initializing the preallocated
>> space, Is this patch acceptable ?
>
> I guess, if Windows truncates the above clusters than file size, it may
> be prefer to truncate on linux too. We really need it over umount?
> We never know the file whether corrupted or preallocated.
>
> And at least for now, it would be better to put under CONFIG_FAT_FALLOC
> or such, and warn it as unofficial way to preallocation on the
> explanation of CONFIG_FAT_FALLOC.
>
> Sorry, I'm still not reviewing the detail of code yet, like locking. And
> I'm still not convinced whether we should add this hack (unofficial way)...
Hi OGAWA.
Sorry for late response.
Currently I am looking for best solution for fallocate support.
next patch will be upated like the following.
1. There is no security bug.
2. Fix issue takes long time while preallocating.
3. No compatibility issue.
Let' discuss after seeing next version's patch again.

Thanks
>
> Thanks.
> --
> OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/