[PATCH 2/3] Btrfs: fix wrong ctime and mtime of a directory after log replay

[PATCH 2/3] Btrfs: fix wrong ctime and mtime of a directory after log replay

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

When replaying a log that contains a new file or directory name that needs
to be added to its parent directory, we end up updating the mtime and the
ctime of the parent directory to the current time after we have set their
values to the correct ones (set at fsync time), efectivelly losing them.

Sample reproducer:

  $ mkfs.btrfs -f /dev/sdb
  $ mount /dev/sdb /mnt

  $ mkdir /mnt/dir
  $ touch /mnt/dir/file

  # fsync of the directory is optional, not needed
  $ xfs_io -c fsync /mnt/dir
  $ xfs_io -c fsync /mnt/dir/file

  $ stat -c %Y /mnt/dir
  1557856079

  <power failure>

  $ sleep 3
  $ mount /dev/sdb /mnt
  $ stat -c %Y /mnt/dir
  1557856082

    --> should have been 1557856079, the mtime is updated to the current
        time when replaying the log

Fix this by not updating the mtime and ctime to the current time at
btrfs_add_link() when we are replaying a log tree.

This could be triggered by my recent fsync fuzz tester for fstests, for
which an fstests patch exists titled "fstests: generic, fsync fuzz tester
with fsstress".

Fixes: e02119d5a7b43 ("Btrfs: Add a write ahead tree log to optimize synchronous operations")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/inode.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index cacde7040329..8e6d7b0bd988 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6381,8 +6381,18 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
 	btrfs_i_size_write(parent_inode, parent_inode->vfs_inode.i_size +
 			   name_len * 2);
 	inode_inc_iversion(&parent_inode->vfs_inode);
-	parent_inode->vfs_inode.i_mtime = parent_inode->vfs_inode.i_ctime =
-		current_time(&parent_inode->vfs_inode);
+	/*
+	 * If we are replaying a log tree, we do not want to update the mtime
+	 * and ctime of the parent directory with the current time, since the
+	 * log replay procedure is responsible for setting them to their correct
+	 * values (the ones it had when the fsync was done).
+	 */
+	if (!test_bit(BTRFS_FS_LOG_RECOVERING, &root->fs_info->flags)) {
+		struct timespec64 now = current_time(&parent_inode->vfs_inode);
+
+		parent_inode->vfs_inode.i_mtime = now;
+		parent_inode->vfs_inode.i_ctime = now;
+	}
 	ret = btrfs_update_inode(trans, root, &parent_inode->vfs_inode);
 	if (ret)
 		btrfs_abort_transaction(trans, ret);
-- 
2.11.0

Re: [PATCH 2/3] Btrfs: fix wrong ctime and mtime of a directory after log replay

[Nikolay Borisov]

On 15.05.19 г. 18:02 ч., fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When replaying a log that contains a new file or directory name that needs
> to be added to its parent directory, we end up updating the mtime and the
> ctime of the parent directory to the current time after we have set their
> values to the correct ones (set at fsync time), efectivelly losing them.
> 
> Sample reproducer:
> 
>   $ mkfs.btrfs -f /dev/sdb
>   $ mount /dev/sdb /mnt
> 
>   $ mkdir /mnt/dir
>   $ touch /mnt/dir/file
> 
>   # fsync of the directory is optional, not needed
>   $ xfs_io -c fsync /mnt/dir
>   $ xfs_io -c fsync /mnt/dir/file
> 
>   $ stat -c %Y /mnt/dir
>   1557856079
> 
>   <power failure>
> 
>   $ sleep 3
>   $ mount /dev/sdb /mnt
>   $ stat -c %Y /mnt/dir
>   1557856082
> 
>     --> should have been 1557856079, the mtime is updated to the current
>         time when replaying the log
> 
> Fix this by not updating the mtime and ctime to the current time at
> btrfs_add_link() when we are replaying a log tree.
> 
> This could be triggered by my recent fsync fuzz tester for fstests, for
> which an fstests patch exists titled "fstests: generic, fsync fuzz tester
> with fsstress".
> 
> Fixes: e02119d5a7b43 ("Btrfs: Add a write ahead tree log to optimize synchronous operations")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/inode.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index cacde7040329..8e6d7b0bd988 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -6381,8 +6381,18 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
>  	btrfs_i_size_write(parent_inode, parent_inode->vfs_inode.i_size +
>  			   name_len * 2);
>  	inode_inc_iversion(&parent_inode->vfs_inode);
> -	parent_inode->vfs_inode.i_mtime = parent_inode->vfs_inode.i_ctime =
> -		current_time(&parent_inode->vfs_inode);
> +	/*
> +	 * If we are replaying a log tree, we do not want to update the mtime
> +	 * and ctime of the parent directory with the current time, since the
> +	 * log replay procedure is responsible for setting them to their correct
> +	 * values (the ones it had when the fsync was done).
> +	 */
> +	if (!test_bit(BTRFS_FS_LOG_RECOVERING, &root->fs_info->flags)) {
> +		struct timespec64 now = current_time(&parent_inode->vfs_inode);
> +
> +		parent_inode->vfs_inode.i_mtime = now;
> +		parent_inode->vfs_inode.i_ctime = now;
> +	}
>  	ret = btrfs_update_inode(trans, root, &parent_inode->vfs_inode);
>  	if (ret)
>  		btrfs_abort_transaction(trans, ret);
> 

Re: [PATCH 2/3] Btrfs: fix wrong ctime and mtime of a directory after log replay

[David Sterba]

On Wed, May 15, 2019 at 04:02:47PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When replaying a log that contains a new file or directory name that needs
> to be added to its parent directory, we end up updating the mtime and the
> ctime of the parent directory to the current time after we have set their
> values to the correct ones (set at fsync time), efectivelly losing them.
> 
> Sample reproducer:
> 
>   $ mkfs.btrfs -f /dev/sdb
>   $ mount /dev/sdb /mnt
> 
>   $ mkdir /mnt/dir
>   $ touch /mnt/dir/file
> 
>   # fsync of the directory is optional, not needed
>   $ xfs_io -c fsync /mnt/dir
>   $ xfs_io -c fsync /mnt/dir/file
> 
>   $ stat -c %Y /mnt/dir
>   1557856079
> 
>   <power failure>
> 
>   $ sleep 3
>   $ mount /dev/sdb /mnt
>   $ stat -c %Y /mnt/dir
>   1557856082
> 
>     --> should have been 1557856079, the mtime is updated to the current
>         time when replaying the log
> 
> Fix this by not updating the mtime and ctime to the current time at
> btrfs_add_link() when we are replaying a log tree.
> 
> This could be triggered by my recent fsync fuzz tester for fstests, for
> which an fstests patch exists titled "fstests: generic, fsync fuzz tester
> with fsstress".
> 
> Fixes: e02119d5a7b43 ("Btrfs: Add a write ahead tree log to optimize synchronous operations")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Added to 5.2-rc queue, thanks.