[PATCH] fat: Support fallocate on fat.

[PATCH] fat: Support fallocate on fat.

[Namjae Jeon]

Implement preallocation via the fallocate syscall on VFAT partitions.
This patch is based on an earlier patch of the same name which had some
issues detailed below and did not get accepted.
Refer https://lkml.org/lkml/2007/12/22/130.

a)The preallocated space was not persistent across remounts when the
FALLOC_FL_KEEP_SIZE flag was set. Also, writes to the file allocated new
clusters instead of using the preallocated area.

Consider the scenario:
mount-->preallocate space for a file --> unmount.
In the old patch,the preallocated space was not reflected for that
file (verified using the 'du' command).

This is now fixed with modifications to fat_fill_inode().

b)There was no need to zero out the clusters when the flag was set.

Instead of doing an expanding truncate, just allocate clusters and add
them to the fat chain. This reduces preallocation time.

Compatibility with windows:
There are no issues when FALLOC_FL_KEEP_SIZE is not set
because it just does an expanding truncate. Thus reading from the
preallocated area on windows returns null until data is written to it.

When a file with preallocated area using the FALLOC_FL_KEEP_SIZE was
written to on windows, the windows driver freed-up the preallocated
clusters and allocated new clusters for the new data. The freed up
clusters gets reflected in the free space available for the partition
which can be seen from the Volume properties.

The windows chkdsk tool also does not report any errors on a
disk containing files with preallocated space.

Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
Signed-off-by: Ravishankar N <cyberax82@gmail.com>
Signed-off-by: Amit Sahrawat <amit.sahrawat83@gmail.com>
---
 fs/fat/file.c  |   85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/fat/inode.c |   10 +++++++
 2 files changed, 95 insertions(+)

diff --git a/fs/fat/file.c b/fs/fat/file.c
index a71fe37..faf3260 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -17,8 +17,12 @@
 #include <linux/blkdev.h>
 #include <linux/fsnotify.h>
 #include <linux/security.h>
+#include <linux/falloc.h>
 #include "fat.h"
 
+static long fat_fallocate(struct file *file, int mode,
+						loff_t offset, loff_t len);
+
 static int fat_ioctl_get_attributes(struct inode *inode, u32 __user *user_attr)
 {
 	u32 attr;
@@ -175,6 +179,7 @@ const struct file_operations fat_file_operations = {
 #endif
 	.fsync		= fat_file_fsync,
 	.splice_read	= generic_file_splice_read,
+	.fallocate		= fat_fallocate,
 };
 
 static int fat_cont_expand(struct inode *inode, loff_t size)
@@ -213,6 +218,86 @@ out:
 	return err;
 }
 
+/*
+ * preallocate space for a file. This implements fat's fallocate file
+ * operation, which gets called from sys_fallocate system call. User
+ * space requests len bytes at offset.If FALLOC_FL_KEEP_SIZE is set
+ * we just allocate clusters without zeroing them out.Otherwise we
+ * allocate and zero out clusters via an expanding truncate.
+ */
+static long fat_fallocate(struct file *file, int mode,
+		loff_t offset, loff_t len)
+{
+	int err = 0;
+	struct inode *inode = file->f_mapping->host;
+	int cluster, nr_cluster, fclus, dclus, free_bytes, nr_bytes;
+	struct super_block *sb = inode->i_sb;
+	struct msdos_sb_info *sbi = MSDOS_SB(sb);
+
+	/* No support for hole punch or other fallocate flags. */
+	if (mode & ~FALLOC_FL_KEEP_SIZE)
+		return -EOPNOTSUPP;
+
+	if ((offset + len) <= MSDOS_I(inode)->mmu_private) {
+		fat_msg(sb, KERN_ERR,
+				"fat_fallocate():Blocks already allocated");
+		return -EINVAL;
+	}
+
+	if ((mode & FALLOC_FL_KEEP_SIZE)) {
+		/* First compute the number of clusters to be allocated */
+		if (inode->i_size > 0) {
+			err = fat_get_cluster(inode, FAT_ENT_EOF,
+					&fclus, &dclus);
+			if (err < 0) {
+				fat_msg(sb, KERN_ERR,
+						"fat_fallocate():fat_get_cluster() error");
+				return err;
+			}
+			free_bytes = ((fclus+1) << sbi->cluster_bits) -
+				(inode->i_size);
+			nr_bytes = (offset + len - inode->i_size) - free_bytes;
+		} else
+			nr_bytes = (offset + len - inode->i_size);
+		nr_cluster = (nr_bytes + (sbi->cluster_size - 1)) >>
+			sbi->cluster_bits;
+		mutex_lock(&inode->i_mutex);
+		/* Start the allocation.We are not zeroing out the clusters */
+		while (nr_cluster-- > 0) {
+			err = fat_alloc_clusters(inode, &cluster, 1);
+			if (err) {
+				fat_msg(sb, KERN_ERR,
+						"fat_fallocate():fat_alloc_clusters() error");
+				goto error;
+			}
+			err = fat_chain_add(inode, cluster, 1);
+			if (err) {
+				fat_free_clusters(inode, cluster);
+				goto error;
+			}
+		}
+		/* update mmu_private to allow writing to allocated clusters */
+		err = fat_get_cluster(inode, FAT_ENT_EOF, &fclus, &dclus);
+		if (err < 0) {
+			fat_msg(sb, KERN_ERR,
+					"fat_fallocate():fat_get_cluster() error");
+			goto error;
+		}
+		MSDOS_I(inode)->mmu_private = (fclus + 1) << sbi->cluster_bits;
+	} else {
+		mutex_lock(&inode->i_mutex);
+		/* This is just an expanding truncate */
+		err = fat_cont_expand(inode, (offset + len));
+		if (err) {
+			fat_msg(sb, KERN_ERR,
+					"fat_fallocate():fat_cont_expand() error");
+		}
+	}
+error:
+	mutex_unlock(&inode->i_mutex);
+	return err;
+}
+
 /* Free all clusters after the skip'th cluster. */
 static int fat_free(struct inode *inode, int skip)
 {
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index fd8e47c..3453218 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -397,8 +397,18 @@ static int fat_fill_inode(struct inode *inode, struct msdos_dir_entry *de)
 	}
 	fat_save_attrs(inode, de->attr);
 
+	/*
+	 * 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);
 	inode->i_blocks = ((inode->i_size + (sbi->cluster_size - 1))
 			   & ~((loff_t)sbi->cluster_size - 1)) >> 9;
+	MSDOS_I(inode)->mmu_private = inode->i_size;
+	/* restore i_size */
+	inode->i_size = le32_to_cpu(de->size);
 
 	fat_time_fat2unix(sbi, &inode->i_mtime, de->time, de->date, 0);
 	if (sbi->options.isvfat) {
-- 
1.7.9.5

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

[OGAWA Hirofumi]

Namjae Jeon <linkinjeon@gmail.com> writes:

> +/*
> + * preallocate space for a file. This implements fat's fallocate file
> + * operation, which gets called from sys_fallocate system call. User
> + * space requests len bytes at offset.If FALLOC_FL_KEEP_SIZE is set
> + * we just allocate clusters without zeroing them out.Otherwise we
> + * allocate and zero out clusters via an expanding truncate.
> + */
> +static long fat_fallocate(struct file *file, int mode,
> +		loff_t offset, loff_t len)
> +{
> +	int err = 0;
> +	struct inode *inode = file->f_mapping->host;
> +	int cluster, nr_cluster, fclus, dclus, free_bytes, nr_bytes;
> +	struct super_block *sb = inode->i_sb;
> +	struct msdos_sb_info *sbi = MSDOS_SB(sb);

What happens if called for directory? And does this guarantee it never
expose the uninitialized data userland?

> +	/* No support for hole punch or other fallocate flags. */
> +	if (mode & ~FALLOC_FL_KEEP_SIZE)
> +		return -EOPNOTSUPP;
>
> +	if ((offset + len) <= MSDOS_I(inode)->mmu_private) {
> +		fat_msg(sb, KERN_ERR,
> +				"fat_fallocate():Blocks already allocated");
> +		return -EINVAL;
> +	}

Please don't output any message by user error. And EINVAL is right
behavior if (offset + len) < allocated size? Sounds like strange design.

> +	if ((mode & FALLOC_FL_KEEP_SIZE)) {
> +		/* First compute the number of clusters to be allocated */
> +		if (inode->i_size > 0) {
> +			err = fat_get_cluster(inode, FAT_ENT_EOF,
> +					&fclus, &dclus);
> +			if (err < 0) {
> +				fat_msg(sb, KERN_ERR,
> +						"fat_fallocate():fat_get_cluster() error");

Use "%s" and __func__. And looks like the error is normal
(e.g. ENOSPC), so I don't see why it needs to report.

[...]

> +	/*
> +	 * 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.

>  	inode->i_blocks = ((inode->i_size + (sbi->cluster_size - 1))
>  			   & ~((loff_t)sbi->cluster_size - 1)) >> 9;
> +	MSDOS_I(inode)->mmu_private = inode->i_size;
> +	/* restore i_size */
> +	inode->i_size = le32_to_cpu(de->size);
>  
>  	fat_time_fat2unix(sbi, &inode->i_mtime, de->time, de->date, 0);
>  	if (sbi->options.isvfat) {

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

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

[Namjae Jeon]

Hi. Ogawa.
2012/7/8, OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>:
> Namjae Jeon <linkinjeon@gmail.com> writes:
>
>> +/*
>> + * preallocate space for a file. This implements fat's fallocate file
>> + * operation, which gets called from sys_fallocate system call. User
>> + * space requests len bytes at offset.If FALLOC_FL_KEEP_SIZE is set
>> + * we just allocate clusters without zeroing them out.Otherwise we
>> + * allocate and zero out clusters via an expanding truncate.
>> + */
>> +static long fat_fallocate(struct file *file, int mode,
>> +		loff_t offset, loff_t len)
>> +{
>> +	int err = 0;
>> +	struct inode *inode = file->f_mapping->host;
>> +	int cluster, nr_cluster, fclus, dclus, free_bytes, nr_bytes;
>> +	struct super_block *sb = inode->i_sb;
>> +	struct msdos_sb_info *sbi = MSDOS_SB(sb);
>
> What happens if called for directory? And does this guarantee it never
> expose the uninitialized data userland?
It cannot be called for directory because in do_fallocate (which calls
fat_fallocate), there is check to open the file in write mode.
If it is opened in read only mode, it returns bad file descriptor:
---------------------------------------------------------------------------------------------------------------------
do_fallocate()
{
        ...
        ..
        if (!(file->f_mode & FMODE_WRITE))
                return -EBADF;
         ....
         ..
-----------------------------------------------------------------------------------------------------------------
We cannot open a directory in write mode. So fallocate can never be
called for a directory.
As long as user appends data to file (instead of seeking to an offset
greater than inode->i_size and writing to it), it can guarantee.
But if user use random offset, it can not..
>
>> +	/* No support for hole punch or other fallocate flags. */
>> +	if (mode & ~FALLOC_FL_KEEP_SIZE)
>> +		return -EOPNOTSUPP;
>>
>> +	if ((offset + len) <= MSDOS_I(inode)->mmu_private) {
>> +		fat_msg(sb, KERN_ERR,
>> +				"fat_fallocate():Blocks already allocated");
>> +		return -EINVAL;
>> +	}
>
> Please don't output any message by user error. And EINVAL is right
> behavior if (offset + len) < allocated size? Sounds like strange design.
Okay, I will remove message.
and I will change return sucess instead of EINVAL.
>
>> +	if ((mode & FALLOC_FL_KEEP_SIZE)) {
>> +		/* First compute the number of clusters to be allocated */
>> +		if (inode->i_size > 0) {
>> +			err = fat_get_cluster(inode, FAT_ENT_EOF,
>> +					&fclus, &dclus);
>> +			if (err < 0) {
>> +				fat_msg(sb, KERN_ERR,
>> +						"fat_fallocate():fat_get_cluster() error");
>
> Use "%s" and __func__. And looks like the error is normal
> (e.g. ENOSPC), so I don't see why it needs to report.
okay, I will remove it.
>
> [...]
>
>> +	/*
>> +	 * 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. 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.

Thanks.
>
>>  	inode->i_blocks = ((inode->i_size + (sbi->cluster_size - 1))
>>  			   & ~((loff_t)sbi->cluster_size - 1)) >> 9;
>> +	MSDOS_I(inode)->mmu_private = inode->i_size;
>> +	/* restore i_size */
>> +	inode->i_size = le32_to_cpu(de->size);
>>
>>  	fat_time_fat2unix(sbi, &inode->i_mtime, de->time, de->date, 0);
>>  	if (sbi->options.isvfat) {
>
> --
> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
>

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

[OGAWA Hirofumi]

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.

> 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.

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

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

[Namjae Jeon]

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 ?

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

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

[OGAWA Hirofumi]

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>

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

[Namjae Jeon]

2012/7/9, 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)...
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@mail.parknet.co.jp>
>

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

[OGAWA Hirofumi]

Namjae Jeon <linkinjeon@gmail.com> writes:

>> 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.

Sure, thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>