linux-ext4 - [PATCH 13/13] xfs: implement lazytime

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: <20210105005452.92521-14-ebiggers@kernel.org>
Date:   Mon,  4 Jan 2021 16:54:52 -0800
From:   Eric Biggers <ebiggers@...nel.org>
To:     linux-fsdevel@...r.kernel.org
Cc:     linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net,
        linux-xfs@...r.kernel.org, Theodore Ts'o <tytso@....edu>,
        Christoph Hellwig <hch@....de>
Subject: [PATCH 13/13] xfs: implement lazytime_expired

From: Eric Biggers <ebiggers@...gle.com>

Implement the new ->lazytime_expired method to get notified of lazytime
timestamp expirations, instead of relying on ->dirty_inode(inode,
I_DIRTY_SYNC) which is potentially ambiguous.

This fixes a bug where XFS didn't write lazytime timestamps to disk upon
a sync(), or after 24 hours (dirtytime_expire_interval * 2).  This is
because it only wrote the timestamps if I_DIRTY_TIME was set in i_state.
But actually when an inode's timestamps expire without the inode being
marked I_DIRTY_SYNC first, then ->dirty_inode isn't called until
__writeback_single_inode() has already cleared I_DIRTY_TIME in i_state.

The new ->lazytime_expired method is unambiguous, so it removes any need
to check for I_DIRTY_TIME, which avoids this bug.

I've written an xfstest which reproduces this bug.

Fixes: c3b1b13190ae ("xfs: implement the lazytime mount option")
Signed-off-by: Eric Biggers <ebiggers@...gle.com>
---
 fs/xfs/xfs_super.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 813be879a5e51..0b7623907b264 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -666,19 +666,13 @@ xfs_fs_destroy_inode(
 }

 static void
-xfs_fs_dirty_inode(
-	struct inode			*inode,
-	int				flag)
+xfs_fs_lazytime_expired(
+	struct inode			*inode)
 {
 	struct xfs_inode		*ip = XFS_I(inode);
 	struct xfs_mount		*mp = ip->i_mount;
 	struct xfs_trans		*tp;

-	if (!(inode->i_sb->s_flags & SB_LAZYTIME))
-		return;
-	if (flag != I_DIRTY_SYNC || !(inode->i_state & I_DIRTY_TIME))
-		return;
-
 	if (xfs_trans_alloc(mp, &M_RES(mp)->tr_fsyncts, 0, 0, 0, &tp))
 		return;
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
@@ -1108,7 +1102,7 @@ xfs_fs_free_cached_objects(
 static const struct super_operations xfs_super_operations = {
 	.alloc_inode		= xfs_fs_alloc_inode,
 	.destroy_inode		= xfs_fs_destroy_inode,
-	.dirty_inode		= xfs_fs_dirty_inode,
+	.lazytime_expired	= xfs_fs_lazytime_expired,
 	.drop_inode		= xfs_fs_drop_inode,
 	.put_super		= xfs_fs_put_super,
 	.sync_fs		= xfs_fs_sync_fs,
-- 
2.30.0