linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Filipe Manana <fdmanana@suse.com>,
	David Sterba <dsterba@suse.com>, Sasha Levin <sashal@kernel.org>
Subject: [PATCH 5.10 04/30] btrfs: fix lost inode on log replay after mix of fsync, rename and inode eviction
Date: Fri,  6 Aug 2021 10:16:42 +0200	[thread overview]
Message-ID: <20210806081113.276899570@linuxfoundation.org> (raw)
In-Reply-To: <20210806081113.126861800@linuxfoundation.org>

From: Filipe Manana <fdmanana@suse.com>

[ Upstream commit ecc64fab7d49c678e70bd4c35fe64d2ab3e3d212 ]

When checking if we need to log the new name of a renamed inode, we are
checking if the inode and its parent inode have been logged before, and if
not we don't log the new name. The check however is buggy, as it directly
compares the logged_trans field of the inodes versus the ID of the current
transaction. The problem is that logged_trans is a transient field, only
stored in memory and never persisted in the inode item, so if an inode
was logged before, evicted and reloaded, its logged_trans field is set to
a value of 0, meaning the check will return false and the new name of the
renamed inode is not logged. If the old parent directory was previously
fsynced and we deleted the logged directory entries corresponding to the
old name, we end up with a log that when replayed will delete the renamed
inode.

The following example triggers the problem:

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

  $ mkdir /mnt/A
  $ mkdir /mnt/B
  $ echo -n "hello world" > /mnt/A/foo

  $ sync

  # Add some new file to A and fsync directory A.
  $ touch /mnt/A/bar
  $ xfs_io -c "fsync" /mnt/A

  # Now trigger inode eviction. We are only interested in triggering
  # eviction for the inode of directory A.
  $ echo 2 > /proc/sys/vm/drop_caches

  # Move foo from directory A to directory B.
  # This deletes the directory entries for foo in A from the log, and
  # does not add the new name for foo in directory B to the log, because
  # logged_trans of A is 0, which is less than the current transaction ID.
  $ mv /mnt/A/foo /mnt/B/foo

  # Now make an fsync to anything except A, B or any file inside them,
  # like for example create a file at the root directory and fsync this
  # new file. This syncs the log that contains all the changes done by
  # previous rename operation.
  $ touch /mnt/baz
  $ xfs_io -c "fsync" /mnt/baz

  <power fail>

  # Mount the filesystem and replay the log.
  $ mount /dev/sdc /mnt

  # Check the filesystem content.
  $ ls -1R /mnt
  /mnt/:
  A
  B
  baz

  /mnt/A:
  bar

  /mnt/B:
  $

  # File foo is gone, it's neither in A/ nor in B/.

Fix this by using the inode_logged() helper at btrfs_log_new_name(), which
safely checks if an inode was logged before in the current transaction.

A test case for fstests will follow soon.

CC: stable@vger.kernel.org # 4.14+
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/btrfs/tree-log.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index d3a2bec931ca..f36928efcf92 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -6456,8 +6456,8 @@ void btrfs_log_new_name(struct btrfs_trans_handle *trans,
 	 * if this inode hasn't been logged and directory we're renaming it
 	 * from hasn't been logged, we don't need to log it
 	 */
-	if (inode->logged_trans < trans->transid &&
-	    (!old_dir || old_dir->logged_trans < trans->transid))
+	if (!inode_logged(trans, inode) &&
+	    (!old_dir || !inode_logged(trans, old_dir)))
 		return;
 
 	btrfs_init_log_ctx(&ctx, &inode->vfs_inode);
-- 
2.30.2




  parent reply	other threads:[~2021-08-06  8:21 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-06  8:16 [PATCH 5.10 00/30] 5.10.57-rc1 review Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 01/30] drm/i915: Revert "drm/i915/gem: Asynchronous cmdparser" Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 02/30] Revert "drm/i915: Propagate errors on awaiting already signaled fences" Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 03/30] btrfs: fix race causing unnecessary inode logging during link and rename Greg Kroah-Hartman
2021-08-06  8:16 ` Greg Kroah-Hartman [this message]
2021-08-06  8:16 ` [PATCH 5.10 05/30] regulator: rtmv20: Fix wrong mask for strobe-polarity-high Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 06/30] regulator: rt5033: Fix n_voltages settings for BUCK and LDO Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 07/30] spi: stm32h7: fix full duplex irq handler handling Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 08/30] ASoC: tlv320aic31xx: fix reversed bclk/wclk master bits Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 09/30] r8152: Fix potential PM refcount imbalance Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 10/30] qed: fix possible unpaired spin_{un}lock_bh in _qed_mcp_cmd_and_union() Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 11/30] ASoC: rt5682: Fix the issue of garbled recording after powerd_dbus_suspend Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 12/30] net: Fix zero-copy head len calculation Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 13/30] ASoC: ti: j721e-evm: Fix unbalanced domain activity tracking during startup Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 14/30] ASoC: ti: j721e-evm: Check for not initialized parent_clk_id Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 15/30] efi/mokvar: Reserve the table only if it is in boot services data Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 16/30] nvme: fix nvme_setup_command metadata trace event Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 17/30] drm/amd/display: Fix comparison error in dcn21 DML Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 18/30] drm/amd/display: Fix max vstartup calculation for modes with borders Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 19/30] ACPI: fix NULL pointer dereference Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 20/30] Revert "spi: mediatek: fix fifo rx mode" Greg Kroah-Hartman
2021-08-06  8:16 ` [PATCH 5.10 21/30] Revert "Bluetooth: Shutdown controller after workqueues are flushed or cancelled" Greg Kroah-Hartman
2021-08-06  8:17 ` [PATCH 5.10 22/30] firmware: arm_scmi: Ensure drivers provide a probe function Greg Kroah-Hartman
2021-08-06  8:17 ` [PATCH 5.10 23/30] firmware: arm_scmi: Add delayed response status check Greg Kroah-Hartman
2021-08-06  8:17 ` [PATCH 5.10 24/30] Revert "watchdog: iTCO_wdt: Account for rebooting on second timeout" Greg Kroah-Hartman
2021-08-06  8:17 ` [PATCH 5.10 25/30] selftests/bpf: Add a test for ptr_to_map_value on stack for helper access Greg Kroah-Hartman
2021-08-06  8:17 ` [PATCH 5.10 26/30] selftest/bpf: Adjust expected verifier errors Greg Kroah-Hartman
2021-08-06  8:17 ` [PATCH 5.10 27/30] bpf, selftests: Adjust few selftest result_unpriv outcomes Greg Kroah-Hartman
2021-08-06  8:17 ` [PATCH 5.10 28/30] bpf: Update selftests to reflect new error states Greg Kroah-Hartman
2021-08-06  8:17 ` [PATCH 5.10 29/30] bpf, selftests: Adjust few selftest outcomes wrt unreachable code Greg Kroah-Hartman
2021-08-06  8:17 ` [PATCH 5.10 30/30] selftest/bpf: Verifier tests for var-off access Greg Kroah-Hartman
2021-08-06 12:59 ` [PATCH 5.10 00/30] 5.10.57-rc1 review Fox Chen
2021-08-06 13:26 ` Pavel Machek
2021-08-06 18:59 ` Guenter Roeck
2021-08-07  6:42 ` Rudi Heitbaum
2021-08-07 10:39 ` Sudip Mukherjee
2021-08-07 18:33 ` Naresh Kamboju
2021-08-07 19:51 ` Aakash Hemadri

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210806081113.276899570@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=dsterba@suse.com \
    --cc=fdmanana@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).