Archive-only list for patches
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	patches@lists.linux.dev, Dave Chinner <dchinner@redhat.com>,
	Christoph Hellwig <hch@lst.de>,
	"Darrick J. Wong" <djwong@kernel.org>,
	Leah Rumancik <leah.rumancik@gmail.com>
Subject: [PATCH 6.1 58/73] xfs: journal geometry is not properly bounds checked
Date: Fri, 27 Sep 2024 14:24:09 +0200	[thread overview]
Message-ID: <20240927121722.271995756@linuxfoundation.org> (raw)
In-Reply-To: <20240927121719.897851549@linuxfoundation.org>

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

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

From: Dave Chinner <dchinner@redhat.com>

[ Upstream commit f1e1765aad7de7a8b8102044fc6a44684bc36180 ]

If the journal geometry results in a sector or log stripe unit
validation problem, it indicates that we cannot set the log up to
safely write to the the journal. In these cases, we must abort the
mount because the corruption needs external intervention to resolve.
Similarly, a journal that is too large cannot be written to safely,
either, so we shouldn't allow those geometries to mount, either.

If the log is too small, we risk having transaction reservations
overruning the available log space and the system hanging waiting
for space it can never provide. This is purely a runtime hang issue,
not a corruption issue as per the first cases listed above. We abort
mounts of the log is too small for V5 filesystems, but we must allow
v4 filesystems to mount because, historically, there was no log size
validity checking and so some systems may still be out there with
undersized logs.

The problem is that on V4 filesystems, when we discover a log
geometry problem, we skip all the remaining checks and then allow
the log to continue mounting. This mean that if one of the log size
checks fails, we skip the log stripe unit check. i.e. we allow the
mount because a "non-fatal" geometry is violated, and then fail to
check the hard fail geometries that should fail the mount.

Move all these fatal checks to the superblock verifier, and add a
new check for the two log sector size geometry variables having the
same values. This will prevent any attempt to mount a log that has
invalid or inconsistent geometries long before we attempt to mount
the log.

However, for the minimum log size checks, we can only do that once
we've setup up the log and calculated all the iclog sizes and
roundoffs. Hence this needs to remain in the log mount code after
the log has been initialised. It is also the only case where we
should allow a v4 filesystem to continue running, so leave that
handling in place, too.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---

Notes:
    A new fix for the latest 6.1.y backport series just came in. Ran
    some tests on it as well and all looks good. Please include with
    the original set.

    Thanks,
    Leah

 fs/xfs/libxfs/xfs_sb.c |   56 ++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/xfs/xfs_log.c       |   47 +++++++++++++----------------------------
 2 files changed, 70 insertions(+), 33 deletions(-)

--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -413,7 +413,6 @@ xfs_validate_sb_common(
 	    sbp->sb_inodelog < XFS_DINODE_MIN_LOG			||
 	    sbp->sb_inodelog > XFS_DINODE_MAX_LOG			||
 	    sbp->sb_inodesize != (1 << sbp->sb_inodelog)		||
-	    sbp->sb_logsunit > XLOG_MAX_RECORD_BSIZE			||
 	    sbp->sb_inopblock != howmany(sbp->sb_blocksize,sbp->sb_inodesize) ||
 	    XFS_FSB_TO_B(mp, sbp->sb_agblocks) < XFS_MIN_AG_BYTES	||
 	    XFS_FSB_TO_B(mp, sbp->sb_agblocks) > XFS_MAX_AG_BYTES	||
@@ -431,6 +430,61 @@ xfs_validate_sb_common(
 		return -EFSCORRUPTED;
 	}
 
+	/*
+	 * Logs that are too large are not supported at all. Reject them
+	 * outright. Logs that are too small are tolerated on v4 filesystems,
+	 * but we can only check that when mounting the log. Hence we skip
+	 * those checks here.
+	 */
+	if (sbp->sb_logblocks > XFS_MAX_LOG_BLOCKS) {
+		xfs_notice(mp,
+		"Log size 0x%x blocks too large, maximum size is 0x%llx blocks",
+			 sbp->sb_logblocks, XFS_MAX_LOG_BLOCKS);
+		return -EFSCORRUPTED;
+	}
+
+	if (XFS_FSB_TO_B(mp, sbp->sb_logblocks) > XFS_MAX_LOG_BYTES) {
+		xfs_warn(mp,
+		"log size 0x%llx bytes too large, maximum size is 0x%llx bytes",
+			 XFS_FSB_TO_B(mp, sbp->sb_logblocks),
+			 XFS_MAX_LOG_BYTES);
+		return -EFSCORRUPTED;
+	}
+
+	/*
+	 * Do not allow filesystems with corrupted log sector or stripe units to
+	 * be mounted. We cannot safely size the iclogs or write to the log if
+	 * the log stripe unit is not valid.
+	 */
+	if (sbp->sb_versionnum & XFS_SB_VERSION_SECTORBIT) {
+		if (sbp->sb_logsectsize != (1U << sbp->sb_logsectlog)) {
+			xfs_notice(mp,
+			"log sector size in bytes/log2 (0x%x/0x%x) must match",
+				sbp->sb_logsectsize, 1U << sbp->sb_logsectlog);
+			return -EFSCORRUPTED;
+		}
+	} else if (sbp->sb_logsectsize || sbp->sb_logsectlog) {
+		xfs_notice(mp,
+		"log sector size in bytes/log2 (0x%x/0x%x) are not zero",
+			sbp->sb_logsectsize, sbp->sb_logsectlog);
+		return -EFSCORRUPTED;
+	}
+
+	if (sbp->sb_logsunit > 1) {
+		if (sbp->sb_logsunit % sbp->sb_blocksize) {
+			xfs_notice(mp,
+		"log stripe unit 0x%x bytes must be a multiple of block size",
+				sbp->sb_logsunit);
+			return -EFSCORRUPTED;
+		}
+		if (sbp->sb_logsunit > XLOG_MAX_RECORD_BSIZE) {
+			xfs_notice(mp,
+		"log stripe unit 0x%x bytes over maximum size (0x%x bytes)",
+				sbp->sb_logsunit, XLOG_MAX_RECORD_BSIZE);
+			return -EFSCORRUPTED;
+		}
+	}
+
 	/* Validate the realtime geometry; stolen from xfs_repair */
 	if (sbp->sb_rextsize * sbp->sb_blocksize > XFS_MAX_RTEXTSIZE ||
 	    sbp->sb_rextsize * sbp->sb_blocksize < XFS_MIN_RTEXTSIZE) {
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -639,7 +639,6 @@ xfs_log_mount(
 	int		num_bblks)
 {
 	struct xlog	*log;
-	bool		fatal = xfs_has_crc(mp);
 	int		error = 0;
 	int		min_logfsbs;
 
@@ -661,53 +660,37 @@ xfs_log_mount(
 	mp->m_log = log;
 
 	/*
-	 * Validate the given log space and drop a critical message via syslog
-	 * if the log size is too small that would lead to some unexpected
-	 * situations in transaction log space reservation stage.
+	 * Now that we have set up the log and it's internal geometry
+	 * parameters, we can validate the given log space and drop a critical
+	 * message via syslog if the log size is too small. A log that is too
+	 * small can lead to unexpected situations in transaction log space
+	 * reservation stage. The superblock verifier has already validated all
+	 * the other log geometry constraints, so we don't have to check those
+	 * here.
 	 *
-	 * Note: we can't just reject the mount if the validation fails.  This
-	 * would mean that people would have to downgrade their kernel just to
-	 * remedy the situation as there is no way to grow the log (short of
-	 * black magic surgery with xfs_db).
+	 * Note: For v4 filesystems, we can't just reject the mount if the
+	 * validation fails.  This would mean that people would have to
+	 * downgrade their kernel just to remedy the situation as there is no
+	 * way to grow the log (short of black magic surgery with xfs_db).
 	 *
-	 * We can, however, reject mounts for CRC format filesystems, as the
+	 * We can, however, reject mounts for V5 format filesystems, as the
 	 * mkfs binary being used to make the filesystem should never create a
 	 * filesystem with a log that is too small.
 	 */
 	min_logfsbs = xfs_log_calc_minimum_size(mp);
-
 	if (mp->m_sb.sb_logblocks < min_logfsbs) {
 		xfs_warn(mp,
 		"Log size %d blocks too small, minimum size is %d blocks",
 			 mp->m_sb.sb_logblocks, min_logfsbs);
-		error = -EINVAL;
-	} else if (mp->m_sb.sb_logblocks > XFS_MAX_LOG_BLOCKS) {
-		xfs_warn(mp,
-		"Log size %d blocks too large, maximum size is %lld blocks",
-			 mp->m_sb.sb_logblocks, XFS_MAX_LOG_BLOCKS);
-		error = -EINVAL;
-	} else if (XFS_FSB_TO_B(mp, mp->m_sb.sb_logblocks) > XFS_MAX_LOG_BYTES) {
-		xfs_warn(mp,
-		"log size %lld bytes too large, maximum size is %lld bytes",
-			 XFS_FSB_TO_B(mp, mp->m_sb.sb_logblocks),
-			 XFS_MAX_LOG_BYTES);
-		error = -EINVAL;
-	} else if (mp->m_sb.sb_logsunit > 1 &&
-		   mp->m_sb.sb_logsunit % mp->m_sb.sb_blocksize) {
-		xfs_warn(mp,
-		"log stripe unit %u bytes must be a multiple of block size",
-			 mp->m_sb.sb_logsunit);
-		error = -EINVAL;
-		fatal = true;
-	}
-	if (error) {
+
 		/*
 		 * Log check errors are always fatal on v5; or whenever bad
 		 * metadata leads to a crash.
 		 */
-		if (fatal) {
+		if (xfs_has_crc(mp)) {
 			xfs_crit(mp, "AAIEEE! Log failed size checks. Abort!");
 			ASSERT(0);
+			error = -EINVAL;
 			goto out_free_log;
 		}
 		xfs_crit(mp, "Log size out of supported range.");



  parent reply	other threads:[~2024-09-27 12:32 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-27 12:23 [PATCH 6.1 00/73] 6.1.112-rc1 review Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 01/73] ASoC: SOF: mediatek: Add missing board compatible Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 02/73] ASoC: allow module autoloading for table db1200_pids Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 03/73] ASoC: allow module autoloading for table board_ids Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 04/73] ALSA: hda/realtek - Fixed ALC256 headphone no sound Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 05/73] ALSA: hda/realtek - FIxed ALC285 " Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 06/73] scsi: lpfc: Fix overflow build issue Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 07/73] pinctrl: at91: make it work with current gpiolib Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 08/73] hwmon: (asus-ec-sensors) remove VRM temp X570-E GAMING Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 09/73] microblaze: dont treat zero reserved memory regions as error Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 10/73] net: ftgmac100: Ensure tx descriptor updates are visible Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 11/73] LoongArch: Define ARCH_IRQ_INIT_FLAGS as IRQ_NOPROBE Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 12/73] wifi: iwlwifi: lower message level for FW buffer destination Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 13/73] wifi: iwlwifi: mvm: fix iwl_mvm_scan_fits() calculation Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 14/73] wifi: iwlwifi: mvm: pause TCM when the firmware is stopped Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 15/73] wifi: iwlwifi: mvm: dont wait for tx queues if firmware is dead Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 16/73] wifi: mac80211: free skb on error path in ieee80211_beacon_get_ap() Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 17/73] wifi: iwlwifi: clear trans->state earlier upon error Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 18/73] can: mcp251xfd: mcp251xfd_ring_init(): check TX-coalescing configuration Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 19/73] ASoC: Intel: soc-acpi-cht: Make Lenovo Yoga Tab 3 X90F DMI match less strict Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 20/73] ASoC: intel: fix module autoloading Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 21/73] ASoC: tda7419: " Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 22/73] spi: spidev: Add an entry for elgin,jg10309-01 Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 23/73] drm: komeda: Fix an issue related to normalized zpos Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 24/73] spi: bcm63xx: Enable module autoloading Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 25/73] smb: client: fix hang in wait_for_response() for negproto Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 26/73] x86/hyperv: Set X86_FEATURE_TSC_KNOWN_FREQ when Hyper-V provides frequency Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 27/73] tools: hv: rm .*.cmd when make clean Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 28/73] block: Fix where bio IO priority gets set Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 29/73] spi: spidev: Add missing spi_device_id for jg10309-01 Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 30/73] ocfs2: add bounds checking to ocfs2_xattr_find_entry() Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 31/73] ocfs2: strict bound check before memcmp in ocfs2_xattr_find_entry() Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 32/73] xfs: dquot shrinker doesnt check for XFS_DQFLAG_FREEING Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 33/73] xfs: Fix deadlock on xfs_inodegc_worker Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 34/73] xfs: fix extent busy updating Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 35/73] xfs: dont use BMBT btree split workers for IO completion Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 36/73] xfs: fix low space alloc deadlock Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 37/73] xfs: prefer free inodes at ENOSPC over chunk allocation Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 38/73] xfs: block reservation too large for minleft allocation Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 39/73] xfs: fix uninitialized variable access Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 40/73] xfs: quotacheck failure can race with background inode inactivation Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 41/73] xfs: fix BUG_ON in xfs_getbmap() Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 42/73] xfs: buffer pins need to hold a buffer reference Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 43/73] xfs: defered work could create precommits Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 44/73] xfs: fix AGF vs inode cluster buffer deadlock Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 45/73] xfs: collect errors from inodegc for unlinked inode recovery Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 46/73] xfs: fix ag count overflow during growfs Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 47/73] xfs: remove WARN when dquot cache insertion fails Greg Kroah-Hartman
2024-09-27 12:23 ` [PATCH 6.1 48/73] xfs: fix the calculation for "end" and "length" Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 49/73] xfs: load uncached unlinked inodes into memory on demand Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 50/73] xfs: fix negative array access in xfs_getbmap Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 51/73] xfs: fix unlink vs cluster buffer instantiation race Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 52/73] xfs: correct calculation for agend and blockcount Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 53/73] xfs: use i_prev_unlinked to distinguish inodes that are not on the unlinked list Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 54/73] xfs: reload entire unlinked bucket lists Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 55/73] xfs: make inode unlinked bucket recovery work with quotacheck Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 56/73] xfs: fix reloading entire unlinked bucket lists Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 57/73] xfs: set bnobt/cntbt numrecs correctly when formatting new AGs Greg Kroah-Hartman
2024-09-27 12:24 ` Greg Kroah-Hartman [this message]
2024-09-27 12:24 ` [PATCH 6.1 59/73] netfilter: nft_socket: make cgroupsv2 matching work with namespaces Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 60/73] netfilter: nft_socket: Fix a NULL vs IS_ERR() bug in nft_socket_cgroup_subtree_level() Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 61/73] netfilter: nft_set_pipapo: walk over current view on netlink dump Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 62/73] netfilter: nf_tables: missing iterator type in lookup walk Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 63/73] Revert "wifi: cfg80211: check wiphy mutex is held for wdev mutex" Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 64/73] gpiolib: cdev: Ignore reconfiguration without direction Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 65/73] gpio: prevent potential speculation leaks in gpio_device_get_desc() Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 66/73] can: mcp251xfd: properly indent labels Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 67/73] can: mcp251xfd: move mcp251xfd_timestamp_start()/stop() into mcp251xfd_chip_start/stop() Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 68/73] selftests: mptcp: join: restrict fullmesh endp on 1st sf Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 69/73] btrfs: calculate the right space for delayed refs when updating global reserve Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 70/73] powercap: RAPL: fix invalid initialization for pl4_supported field Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 71/73] x86/mm: Switch to new Intel CPU model defines Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 72/73] USB: serial: pl2303: add device id for Macrosilicon MS3020 Greg Kroah-Hartman
2024-09-27 12:24 ` [PATCH 6.1 73/73] USB: usbtmc: prevent kernel-usb-infoleak Greg Kroah-Hartman
2024-09-27 15:17 ` [PATCH 6.1 00/73] 6.1.112-rc1 review Peter Schneider
2024-09-27 15:52 ` Allen
2024-09-27 18:35 ` Jon Hunter
2024-09-27 18:40 ` Florian Fainelli
2024-09-28 12:39 ` Naresh Kamboju
2024-09-28 17:13 ` Shuah Khan
2024-09-29  8:43 ` Ron Economos
2024-09-29 11:19 ` Muhammad Usama Anjum
2024-09-30  8:41 ` Pavel Machek

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=20240927121722.271995756@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=dchinner@redhat.com \
    --cc=djwong@kernel.org \
    --cc=hch@lst.de \
    --cc=leah.rumancik@gmail.com \
    --cc=patches@lists.linux.dev \
    --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