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

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

Namjae Jeon <linkinjeon@gmail.com> writes:

> 2012/7/9, OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>:
>> Namjae Jeon <linkinjeon@gmail.com> 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)...

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