On Thu, May 16, 2019 at 03:48:55PM +0100, fdmanana@xxxxxxxxxx wrote:
> From: Filipe Manana <fdmanana@xxxxxxxx>
>
> While logging an inode we follow its ancestors and for each one we mark
> it as logged in the current transaction, even if we have not logged it.
> As a consequence if we change an attribute of an ancestor, such as the
> UID or GID for example, and then explicitly fsync it, we end up not
> logging the inode at all despite returning success to user space, which
> results in the attribute being lost if a power failure happens after
> the fsync.
>
> Sample reproducer:
>
> $ mkfs.btrfs -f /dev/sdb
> $ mount /dev/sdb /mnt
>
> $ mkdir /mnt/dir
> $ chown 6007:6007 /mnt/dir
>
> $ sync
>
> $ chown 9003:9003 /mnt/dir
> $ touch /mnt/dir/file
> $ xfs_io -c fsync /mnt/dir/file
>
> # fsync our directory after fsync'ing the new file, should persist the
> # new values for the uid and gid.
> $ xfs_io -c fsync /mnt/dir
>
> <power failure>
>
> $ mount /dev/sdb /mnt
> $ stat -c %u:%g /mnt/dir
> 6007:6007
>
> --> should be 9003:9003, the uid and gid were not persisted, despite
> the explicit fsync on the directory prior to the power failure
>
> Fix this by not updating the logged_trans field of ancestor inodes when
> logging an inode, since we have not logged them. Let only future calls to
> btrfs_log_inode() to mark inodes as logged.
>
> 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: 12fcfd22fe5b ("Btrfs: tree logging unlink/rename fixes")
> Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
> ---
Added to 5.2-rc queue, thanks.
