linux-ext4 - Re: [PATCH] Add flag to files with blocks intentionally past EOF

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-id: <22655F73-6EB0-40AC-9D51-794F6764CF67@sun.com>
Date:	Thu, 21 Jan 2010 17:32:22 -0700
From:	Andreas Dilger <adilger@....com>
To:	"Aneesh Kumar K. V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc:	Eric Sandeen <sandeen@...hat.com>,
	ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] Add flag to files with blocks intentionally past EOF

On 2010-01-20, at 02:03, Aneesh Kumar K. V wrote:
>> @@ -4741,8 +4743,8 @@ void ext4_get_inode_flags(struct  
>> ext4_inode_info *ei)
>> {
>> 	unsigned int flags = ei->vfs_inode.i_flags;
>>
>> -	ei->i_flags &= ~(EXT4_SYNC_FL|EXT4_APPEND_FL|
>> -			EXT4_IMMUTABLE_FL|EXT4_NOATIME_FL|EXT4_DIRSYNC_FL);
>> +	ei->i_flags &= ~(EXT4_SYNC_FL|EXT4_APPEND_FL|EXT4_IMMUTABLE_FL|
>> +
>> EXT4_NOATIME_FL|EXT4_DIRSYNC_FL|EXT4_EOFBLOCKS_FL);
>
>
> Do we really need to allow the get and set of this flag. IMHO a  
> truncate
> should be the only API and the flag should be remove implicitly for  
> that.

Since this flag is set on disk in the inode flags, it makes sense to  
allow clearing it via "chattr", just like "chattr +e" will remap a  
file to extent format.  The risk with setting it via truncate() is  
that this is racy with some other process writing to the file.  If we  
allow setting it via "chattr" this can be done in a non-racy manner,  
by locking the inode and dropping only the blocks beyond EOF.

>> @@ -92,6 +92,16 @@ long ext4_ioctl(struct file *filp, unsigned int  
>> cmd, unsigned long arg)
>> 			flags &= ~EXT4_EXTENTS_FL;
>> 		}
>>
>> +		if (flags & EXT4_EOFBLOCKS_FL) {
>> +			/* we don't support adding EOFBLOCKS flag */
>> +			if (!(oldflags & EXT4_EOFBLOCKS_FL)) {
>> +				err = -EOPNOTSUPP;
>> +				goto flags_out;
>> +			}
>> +		} else if (oldflags & EXT4_EOFBLOCKS_FL)
>> +			/* free the space reserved with fallocate KEEPSIZE */
>> +			vmtruncate(inode, inode->i_size);
>> +
>> 		handle = ext4_journal_start(inode, 1);
>> 		if (IS_ERR(handle)) {
>> 			err = PTR_ERR(handle);
>
> If we remove ext4_ioctl support i guess that patch can become much
> simpler.


Sure it will be simpler, but less useful.  The point of exposing this  
flag via lsattr is to allow userspace to determine which files are  
holding blocks beyond EOF, so that if the filesystem is getting too  
full it is possible to run e.g. "lsattr -R" and find files with this  
EOF attribute and truncate them.  Without keeping at least the  
EOFBLOCKS flag in USER_VISIBLE this is impossible.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html