linux-ext4 - Re: Question on fallocate/ftruncate sequence

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: <20090830025250.GA25768@mit.edu>
Date:	Sat, 29 Aug 2009 22:52:50 -0400
From:	Theodore Tso <tytso@....edu>
To:	Jiaying Zhang <jiayingz@...gle.com>
Cc:	Andreas Dilger <adilger@....com>,
	Frank Mayhar <fmayhar@...gle.com>,
	Eric Sandeen <sandeen@...hat.com>,
	Curt Wohlgemuth <curtw@...gle.com>,
	ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: Question on fallocate/ftruncate sequence

On Fri, Aug 28, 2009 at 05:40:54PM -0700, Jiaying Zhang wrote:
> --- .pc/fallocate_keepsizse.patch/fs/attr.c	2009-08-28 15:38:46.000000000 -0700
> +++ fs/attr.c	2009-08-28 17:01:04.000000000 -0700
> @@ -68,7 +68,8 @@ int inode_setattr(struct inode * inode,
>  	unsigned int ia_valid = attr->ia_valid;
> 
>  	if (ia_valid & ATTR_SIZE &&
> -	    (attr->ia_size != i_size_read(inode)) {
> +	    (attr->ia_size != i_size_read(inode) ||
> +	     (inode->i_flags & FS_KEEPSIZE_FL))) {
>  		int error = vmtruncate(inode, attr->ia_size);
>  		if (error)
>  			return error;

Instead of doing this in the generic code, it really should be done in
ext4_setattr.  Technically speaking, we don't actually need the
FS_KEEPSIZE_FL to solve this problem; instead we can simply have the
ext4 code look in the extent tree to see if there are any blocks
mapped beyond the logical block:

       i_size_read(inode) >> inode->i_sb->s_blocksize_bits

Having a flag as Andreas suggested does help with the issue of e2fsck
noticing whether or not i_size is incorrect (and should be fixed) or
the file has been extended.  So keeping having the flag is an OK thing
to do, but we need to be careful about a particularly subtle
overloading problem.  The flags FS_*_FL as defined in
include/linux/fs.h are technically only for in-memory use.  The ext4
on-disk format flags is EXT4_*_FL, and defined in ext4.h.

The flags were originially defined for use in ext2/3/4, but later on
other filesystems adopted those flags so that e2fsprogs's chattr and
lsattr programs could be used for their filesystems as well.  It just
so happens that for ext2/3/4 the on-disk encoding of those flags in
the in-memory encoding of those flags in i_flags are the same, but
that means that the flags need to be defined in both places to avoid
assignment overlaps.  We also need to be clear whether the flags are
internal flags for ext4's use only, or flags meant for use by all
filesystems.  This is why the testing for FS_KEEPSIZE_FL in fs/attr is
particularly bad, if the flag are going to be set in fs/ext4/extents.c.

It's better to define the flag as EXT4_KEEPSIZE_FL, and to use it as
EXT4_KEEPSIZE_FL, but make a note of that bitfield position as being
reserved in include/linux/fs.h.

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