[3/3] xfs: log is not writable if we have unknown rocompat features

Commit Message

Darrick J. Wong Aug. 24, 2023, 11:21 p.m. UTC

From: Darrick J. Wong <[e-mail]>

Ever since commit 9e037cb7972f, the superblock write verifier will trip
if someone tries to write a superblock with unknown rocompat features.
However, we allow ro mounts of a filesystem with unknown rocompat
features if the log is clean, except that has been broken for years
because the end of an ro mount cleans the log, which logs and writes the
superblock.

Therefore, don't allow log writes to happen if there are unknown
rocompat features set.

Fixes: 9e037cb7972f ("xfs: check for unknown v5 feature bits in superblock write verifier")
Signed-off-by: Darrick J. Wong <[e-mail]>
---
 fs/xfs/xfs_log.c         |    6 ++++++
 fs/xfs/xfs_log_priv.h    |    7 +++++++
 fs/xfs/xfs_log_recover.c |    7 +++++++
 3 files changed, 20 insertions(+)

Comments

Dave Chinner Aug. 25, 2023, 1:08 a.m. UTC | #1

On Thu, Aug 24, 2023 at 04:21:52PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <[e-mail]>
> 
> Ever since commit 9e037cb7972f, the superblock write verifier will trip
> if someone tries to write a superblock with unknown rocompat features.
> However, we allow ro mounts of a filesystem with unknown rocompat
> features if the log is clean, except that has been broken for years
> because the end of an ro mount cleans the log, which logs and writes the
> superblock.
> 
> Therefore, don't allow log writes to happen if there are unknown
> rocompat features set.
> 
> Fixes: 9e037cb7972f ("xfs: check for unknown v5 feature bits in superblock write verifier")
> Signed-off-by: Darrick J. Wong <[e-mail]>

Yup, this makes sense.

Reviewed-by: Dave Chinner <[e-mail]>

Patch

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 51c100c86177..c1bbc8040bcb 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -391,6 +391,8 @@  xfs_log_writable(
 		return false;
 	if (xlog_is_shutdown(mp->m_log))
 		return false;
+	if (xlog_is_readonly(mp->m_log))
+		return false;
 	return true;
 }
 
@@ -408,6 +410,8 @@  xfs_log_regrant(
 
 	if (xlog_is_shutdown(log))
 		return -EIO;
+	if (xlog_is_readonly(log))
+		return -EROFS;
 
 	XFS_STATS_INC(mp, xs_try_logspace);
 
@@ -471,6 +475,8 @@  xfs_log_reserve(
 
 	if (xlog_is_shutdown(log))
 		return -EIO;
+	if (xlog_is_readonly(log))
+		return -EROFS;
 
 	XFS_STATS_INC(mp, xs_try_logspace);
 
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index af87648331d5..14892e01de38 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -461,6 +461,7 @@  struct xlog {
 #define XLOG_IO_ERROR		2	/* log hit an I/O error, and being
 				   shutdown */
 #define XLOG_TAIL_WARN		3	/* log tail verify warning issued */
+#define XLOG_READONLY		4	/* cannot write to the log */
 
 static inline bool
 xlog_recovery_needed(struct xlog *log)
@@ -480,6 +481,12 @@  xlog_is_shutdown(struct xlog *log)
 	return test_bit(XLOG_IO_ERROR, &log->l_opstate);
 }
 
+static inline bool
+xlog_is_readonly(struct xlog *log)
+{
+	return test_bit(XLOG_READONLY, &log->l_opstate);
+}
+
 /*
  * Wait until the xlog_force_shutdown() has marked the log as shut down
  * so xlog_is_shutdown() will always return true.
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index b4458b7fd6f7..f8f13d5f79cd 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -3450,6 +3450,13 @@  xlog_recover(
 
 		error = xlog_do_recover(log, head_blk, tail_blk);
 		set_bit(XLOG_RECOVERY_NEEDED, &log->l_opstate);
+	} else if (unknown_rocompat) {
+		/*
+		 * Log recovery wasn't needed, but if the superblock has
+		 * unknown rocompat features, don't allow log writes at all
+		 * because the sb write verifier will trip.
+		 */
+		set_bit(XLOG_READONLY, &log->l_opstate);
 	}
 	return error;
 }