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, stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Brian Foster <bfoster@redhat.com>,
	"Darrick J. Wong" <darrick.wong@oracle.com>
Subject: [PATCH 4.13 32/52] xfs: fix log recovery corruption error due to tail overwrite
Date: Mon, 18 Sep 2017 11:10:00 +0200	[thread overview]
Message-ID: <20170918090908.797336058@linuxfoundation.org> (raw)
In-Reply-To: <20170918090904.072766209@linuxfoundation.org>

4.13-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Brian Foster <bfoster@redhat.com>

commit 4a4f66eac4681378996a1837ad1ffec3a2e2981f upstream.

If we consider the case where the tail (T) of the log is pinned long
enough for the head (H) to push and block behind the tail, we can
end up blocked in the following state without enough free space (f)
in the log to satisfy a transaction reservation:

	0	phys. log	N
	[-------HffT---H'--T'---]

The last good record in the log (before H) refers to T. The tail
eventually pushes forward (T') leaving more free space in the log
for writes to H. At this point, suppose space frees up in the log
for the maximum of 8 in-core log buffers to start flushing out to
the log. If this pushes the head from H to H', these next writes
overwrite the previous tail T. This is safe because the items logged
from T to T' have been written back and removed from the AIL.

If the next log writes (H -> H') happen to fail and result in
partial records in the log, the filesystem shuts down having
overwritten T with invalid data. Log recovery correctly locates H on
the subsequent mount, but H still refers to the now corrupted tail
T. This results in log corruption errors and recovery failure.

Since the tail overwrite results from otherwise correct runtime
behavior, it is up to log recovery to try and deal with this
situation. Update log recovery tail verification to run a CRC pass
from the first record past the tail to the head. This facilitates
error detection at T and moves the recovery tail to the first good
record past H' (similar to truncating the head on torn write
detection). If corruption is detected beyond the range possibly
affected by the max number of iclogs, the log is legitimately
corrupted and log recovery failure is expected.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/xfs/xfs_log_recover.c |  108 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 77 insertions(+), 31 deletions(-)

--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -1029,61 +1029,106 @@ out_error:
 }
 
 /*
- * Check the log tail for torn writes. This is required when torn writes are
- * detected at the head and the head had to be walked back to a previous record.
- * The tail of the previous record must now be verified to ensure the torn
- * writes didn't corrupt the previous tail.
+ * Calculate distance from head to tail (i.e., unused space in the log).
+ */
+static inline int
+xlog_tail_distance(
+	struct xlog	*log,
+	xfs_daddr_t	head_blk,
+	xfs_daddr_t	tail_blk)
+{
+	if (head_blk < tail_blk)
+		return tail_blk - head_blk;
+
+	return tail_blk + (log->l_logBBsize - head_blk);
+}
+
+/*
+ * Verify the log tail. This is particularly important when torn or incomplete
+ * writes have been detected near the front of the log and the head has been
+ * walked back accordingly.
+ *
+ * We also have to handle the case where the tail was pinned and the head
+ * blocked behind the tail right before a crash. If the tail had been pushed
+ * immediately prior to the crash and the subsequent checkpoint was only
+ * partially written, it's possible it overwrote the last referenced tail in the
+ * log with garbage. This is not a coherency problem because the tail must have
+ * been pushed before it can be overwritten, but appears as log corruption to
+ * recovery because we have no way to know the tail was updated if the
+ * subsequent checkpoint didn't write successfully.
  *
- * Return an error if CRC verification fails as recovery cannot proceed.
+ * Therefore, CRC check the log from tail to head. If a failure occurs and the
+ * offending record is within max iclog bufs from the head, walk the tail
+ * forward and retry until a valid tail is found or corruption is detected out
+ * of the range of a possible overwrite.
  */
 STATIC int
 xlog_verify_tail(
 	struct xlog		*log,
 	xfs_daddr_t		head_blk,
-	xfs_daddr_t		tail_blk)
+	xfs_daddr_t		*tail_blk,
+	int			hsize)
 {
 	struct xlog_rec_header	*thead;
 	struct xfs_buf		*bp;
 	xfs_daddr_t		first_bad;
-	int			count;
 	int			error = 0;
 	bool			wrapped;
-	xfs_daddr_t		tmp_head;
+	xfs_daddr_t		tmp_tail;
+	xfs_daddr_t		orig_tail = *tail_blk;
 
 	bp = xlog_get_bp(log, 1);
 	if (!bp)
 		return -ENOMEM;
 
 	/*
-	 * Seek XLOG_MAX_ICLOGS + 1 records past the current tail record to get
-	 * a temporary head block that points after the last possible
-	 * concurrently written record of the tail.
+	 * Make sure the tail points to a record (returns positive count on
+	 * success).
 	 */
-	count = xlog_seek_logrec_hdr(log, head_blk, tail_blk,
-				     XLOG_MAX_ICLOGS + 1, bp, &tmp_head, &thead,
-				     &wrapped);
-	if (count < 0) {
-		error = count;
+	error = xlog_seek_logrec_hdr(log, head_blk, *tail_blk, 1, bp,
+			&tmp_tail, &thead, &wrapped);
+	if (error < 0)
 		goto out;
-	}
+	if (*tail_blk != tmp_tail)
+		*tail_blk = tmp_tail;
 
 	/*
-	 * If the call above didn't find XLOG_MAX_ICLOGS + 1 records, we ran
-	 * into the actual log head. tmp_head points to the start of the record
-	 * so update it to the actual head block.
+	 * Run a CRC check from the tail to the head. We can't just check
+	 * MAX_ICLOGS records past the tail because the tail may point to stale
+	 * blocks cleared during the search for the head/tail. These blocks are
+	 * overwritten with zero-length records and thus record count is not a
+	 * reliable indicator of the iclog state before a crash.
 	 */
-	if (count < XLOG_MAX_ICLOGS + 1)
-		tmp_head = head_blk;
-
-	/*
-	 * We now have a tail and temporary head block that covers at least
-	 * XLOG_MAX_ICLOGS records from the tail. We need to verify that these
-	 * records were completely written. Run a CRC verification pass from
-	 * tail to head and return the result.
-	 */
-	error = xlog_do_recovery_pass(log, tmp_head, tail_blk,
+	first_bad = 0;
+	error = xlog_do_recovery_pass(log, head_blk, *tail_blk,
 				      XLOG_RECOVER_CRCPASS, &first_bad);
+	while (error == -EFSBADCRC && first_bad) {
+		int	tail_distance;
+
+		/*
+		 * Is corruption within range of the head? If so, retry from
+		 * the next record. Otherwise return an error.
+		 */
+		tail_distance = xlog_tail_distance(log, head_blk, first_bad);
+		if (tail_distance > BTOBB(XLOG_MAX_ICLOGS * hsize))
+			break;
+
+		/* skip to the next record; returns positive count on success */
+		error = xlog_seek_logrec_hdr(log, head_blk, first_bad, 2, bp,
+				&tmp_tail, &thead, &wrapped);
+		if (error < 0)
+			goto out;
+
+		*tail_blk = tmp_tail;
+		first_bad = 0;
+		error = xlog_do_recovery_pass(log, head_blk, *tail_blk,
+					      XLOG_RECOVER_CRCPASS, &first_bad);
+	}
 
+	if (!error && *tail_blk != orig_tail)
+		xfs_warn(log->l_mp,
+		"Tail block (0x%llx) overwrite detected. Updated to 0x%llx",
+			 orig_tail, *tail_blk);
 out:
 	xlog_put_bp(bp);
 	return error;
@@ -1187,7 +1232,8 @@ xlog_verify_head(
 	if (error)
 		return error;
 
-	return xlog_verify_tail(log, *head_blk, *tail_blk);
+	return xlog_verify_tail(log, *head_blk, tail_blk,
+				be32_to_cpu((*rhead)->h_size));
 }
 
 /*

  parent reply	other threads:[~2017-09-18  9:11 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-18  9:09 [PATCH 4.13 00/52] 4.13.3-stable review Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 01/52] Revert "net: use lib/percpu_counter API for fragmentation mem accounting" Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 02/52] Revert "net: fix percpu memory leaks" Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 03/52] gianfar: Fix Tx flow control deactivation Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 04/52] vhost_net: correctly check tx avail during rx busy polling Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 05/52] ip6_gre: update mtu properly in ip6gre_err Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 06/52] udp: drop head states only when all skb references are gone Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 07/52] ipv6: fix memory leak with multiple tables during netns destruction Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 08/52] ipv6: fix typo in fib6_net_exit() Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 09/52] sctp: fix missing wake ups in some situations Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 10/52] tcp: fix a request socket leak Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 11/52] ip_tunnel: fix setting ttl and tos value in collect_md mode Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 12/52] f2fs: let fill_super handle roll-forward errors Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 13/52] f2fs: check hot_data for roll-forward recovery Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 14/52] thunderbolt: Remove superfluous check Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 15/52] thunderbolt: Make key root-only accessible Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 16/52] thunderbolt: Allow clearing the key Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 17/52] x86/fsgsbase/64: Fully initialize FS and GS state in start_thread_common Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 18/52] x86/fsgsbase/64: Report FSBASE and GSBASE correctly in core dumps Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 19/52] x86/switch_to/64: Rewrite FS/GS switching yet again to fix AMD CPUs Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 20/52] x86/mm, mm/hwpoison: Clear PRESENT bit for kernel 1:1 mappings of poison pages Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 21/52] ovl: fix false positive ESTALE on lookup Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 22/52] fuse: allow server to run in different pid_ns Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 23/52] idr: remove WARN_ON_ONCE() when trying to replace negative ID Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 24/52] libnvdimm, btt: check memory allocation failure Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 25/52] libnvdimm: fix integer overflow static analysis warning Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 26/52] xfs: write unmount record for ro mounts Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 27/52] xfs: toggle readonly state around xfs_log_mount_finish Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 28/52] xfs: Add infrastructure needed for error propagation during buffer IO failure Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 29/52] xfs: Properly retry failed inode items in case of error during buffer writeback Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 30/52] xfs: fix recovery failure when log record header wraps log end Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 31/52] xfs: always verify the log tail during recovery Greg Kroah-Hartman
2017-09-18  9:10 ` Greg Kroah-Hartman [this message]
2017-09-18  9:10 ` [PATCH 4.13 33/52] xfs: handle -EFSCORRUPTED during head/tail verification Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 34/52] xfs: stop searching for free slots in an inode chunk when there are none Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 35/52] xfs: evict all inodes involved with log redo item Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 36/52] xfs: check for race with xfs_reclaim_inode() in xfs_ifree_cluster() Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 37/52] xfs: open-code xfs_buf_item_dirty() Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 38/52] xfs: remove unnecessary dirty bli format check for ordered bufs Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 39/52] xfs: ordered buffer log items are never formatted Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 40/52] xfs: refactor buffer logging into buffer dirtying helper Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 41/52] xfs: dont log dirty ranges for ordered buffers Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 42/52] xfs: skip bmbt block ino validation during owner change Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 43/52] xfs: move bmbt owner change to last step of extent swap Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 44/52] xfs: disallow marking previously dirty buffers as ordered Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 45/52] xfs: relog dirty buffers during swapext bmbt owner change Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 46/52] xfs: disable per-inode DAX flag Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 47/52] xfs: fix incorrect log_flushed on fsync Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 48/52] xfs: dont set v3 xflags for v2 inodes Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 49/52] xfs: open code end_buffer_async_write in xfs_finish_page_writeback Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 50/52] xfs: use kmem_free to free return value of kmem_zalloc Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 51/52] md/raid1/10: reset bio allocated from mempool Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 52/52] md/raid5: release/flush io in raid5_do_work() Greg Kroah-Hartman
2017-09-18 19:29 ` [PATCH 4.13 00/52] 4.13.3-stable review Guenter Roeck
2017-09-18 20:17 ` Shuah Khan
2017-09-19  6:33   ` Greg Kroah-Hartman

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=20170918090908.797336058@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=bfoster@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=linux-kernel@vger.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).