linux-ext4 - Re: [PATCH] ext4: turn on i_version updates by default

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]

Date:	Mon, 14 May 2012 17:33:04 -0600
From:	Andreas Dilger <adilger@...ger.ca>
To:	"Myklebust, Trond" <Trond.Myklebust@...app.com>
Cc:	"J. Bruce Fields" <bfields@...ldses.org>,
	"Theodore Ts'o" <tytso@....edu>,
	"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
	"linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH] ext4: turn on i_version updates by default

On 2012-05-14, at 5:08 PM, Myklebust, Trond wrote:
> On Mon, 2012-05-14 at 09:02 -0600, Andreas Dilger wrote:
>> On 2012-05-14, at 8:06, "J. Bruce Fields" <bfields@...ldses.org> wrote:
>>> knfsd needs i_version updates on, as will userspace nfs servers and
>>> probably others.
>>> 
>>> The only effects are that inode->i_version is bumped (under the i_lock)
>>> in more places, and that ->dirty_inode(I_DIRTY_DATASYNC) may be called
>>> more frequently than once per jiffy on write (see file_update_time).
>>> However the latter appears to be mostly a no-op in that case.
>> 
>> I thought this can have noticeable performance impact, since ext4_mark_inode_dirty() is quite heavyweight?
>> 
>> This is one of the reasons that the i_version update is conditional.
>> If someone is exporting a filesystem from userspace the should be
>> able to turn this on as a mount option, and knfsd could do it from inside the kernel. Why add overhead when it is not needed?
> 
> No. Having knfsd doing something like that under the covers is a BAD
> idea. It is way too easy to get into situations where someone starts
> changing files after the mount and before knfsd is started. As soon as
> you allow files to be changed without i_version changing, then you are
> setting yourself up for future corruption.
> 
> Ideally, an NFSv4-exported filesystem should be required to set the
> tune2fs mount_opts to include the 'i_version' flag to make it hard to
> inadvertently mount that filesystem without it.

I said as much in another reply - that once i_version is used on
a filesystem, it should be made "sticky" (i.e. permanently enabled
for that filesystem).  However, until that time it shouldn't be
enabled just because it might one day be used.

Even better than just blindly bumping the i_version on every change,
it would be better to have users of i_version (i.e. knfsd) flag the
inode with "needs i_version update" then read the version.  When the
filesystem/VFS bumps i_version the next time it can clear this flag
and not update i_version again until after the next time i_version
is actually used.

Cheers, Andreas





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