linux-kernel - Re: [PATCH v3] fat: editions to support fat

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAKYAXd_E8ZzutRVX6bCpzuosVLqsCDk0VzPYeXtcG8K+4wwBvQ@mail.gmail.com>
Date:	Mon, 11 Mar 2013 18:52:25 +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, abartlet@...ba.org,
	Namjae Jeon <namjae.jeon@...sung.com>,
	Ravishankar N <ravi.n1@...sung.com>
Subject: Re: [PATCH v3] fat: editions to support fat_fallocate

2013/3/9, OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>:
> Namjae Jeon <linkinjeon@...il.com> writes:
>
>>  static int fat_file_release(struct inode *inode, struct file *filp)
>>  {
>> +	struct super_block *sb = inode->i_sb;
>> +	loff_t mmu_private_ideal = (inode->i_size + (sb->s_blocksize-1)) &
>> +				    ~(sb->s_blocksize-1);
>> +	if (mmu_private_ideal < MSDOS_I(inode)->mmu_private &&
>> +	    filp->f_dentry->d_count == 1)
>> +		fat_truncate_blocks(inode, inode->i_size);
>
Hi OGAWA.
> Without locking, truncate is racy.
I will check for races and provide locking.
>
> This choose ->release(). BTW, we would also be able to do this only
> ->evict_inode(), although I'm not thinking yet which one is better.
>
> If you had conclusion, it would be nice to explain it.
evict_inode() will be called only when we unlink the file or if inode
is evicted from cache.
As we discussed with you before, We considered preallocated blocks is
discarded on all close file cases(unlink and muliple openning file).
So we think it would be better to do this in ->release().
>
>> +static long fat_fallocate(struct file *file, int mode,
>> +				loff_t offset, loff_t len)
>> +{
>>
>> +	if ((offset + len) <= MSDOS_I(inode)->mmu_private) {
>> +		fat_msg(sb, KERN_ERR,
>> +			"fat_fallocate():Blocks already allocated");
>> +		return -EINVAL;
>> +	}
>
> Also this looks like totally racy.
Okay, I will fix it.

>
>>  static int fat_write_begin(struct file *file, struct address_space
>> *mapping,
>>  			loff_t pos, unsigned len, unsigned flags,
>>  			struct page **pagep, void **fsdata)
>>  {
>>  	int err;
>> +	struct inode *inode = mapping->host;
>> +	struct super_block *sb = inode->i_sb;
>> +	loff_t mmu_private_actual = MSDOS_I(inode)->mmu_private;
>> +	loff_t mmu_private_ideal = (inode->i_size + (sb->s_blocksize-1)) &
>> +					 ~(sb->s_blocksize-1);
>> +
>> +	if ((mmu_private_actual > mmu_private_ideal) && (pos > inode->i_size))
>> {
>> +		err = fat_zero_falloc_area(file, mapping, pos);
>> +		if (err)
>> +			fat_msg(sb, KERN_ERR, "error zeroing fallocated area");
>> +	}
>>
>>  	*pagep = NULL;
>>  	err = cont_write_begin(file, mapping, pos, len, flags,
>
> Hm, only write_begin is enough to handle mmap, truncate, and etc.?
We had not checked these use cases.  We will check these.

Thanks for review!

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