[PATCH v2 04/34] ext2: don't set SB_RDONLY after filesystem errors

[Christian Brauner <brauner@kernel.org>]

With errors=remount-ro ext2_error() sets SB_RDONLY from whatever context
hit the error and without holding s_umount. That races with everyone
else changing sb->s_flags and it breaks freezing. If it happens while
the filesystem is frozen thaw_super() sees a read-only superblock and
leaves the s_writers rwsems locked so every writer hangs. A lookup in a
corrupted directory is enough. Karl proposed teaching thaw_super() about
it but Jan preferred fixing the filesystems (cf. [1]) like ext4 and f2fs
already did.

So call sb_set_emergency_ro() instead and let the VFS refuse
modifications. ext2 still refuses allocations, ext2_write_inode(),
ext2_sync_fs() and ext2_xattr_set() itself. The superblock isn't written
after the error is recorded and unlinked inodes are left to e2fsck.
ext2_handle_dirsync() treats EROFS as success in that state because the
entry was already changed.

A remount to read-write fails until umount and writes through file
descriptors opened before the error fail with EROFS.

[1]: https://lore.kernel.org/r/hyhuckyohsabx2suvq3z7uy3pbgmihva6wgzxr3op7pzcrlu33@p7zzlgp2ehjf

Reported-by: Karl Mehltretter <kmehltretter@gmail.com>
Link: https://lore.kernel.org/r/20260903060855.4610-1-kmehltretter@gmail.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/ext2/balloc.c |  5 +++++
 fs/ext2/dir.c    |  3 +++
 fs/ext2/ialloc.c |  2 ++
 fs/ext2/inode.c  | 10 ++++++++--
 fs/ext2/super.c  | 24 ++++++++++++++++++------
 fs/ext2/xattr.c  |  2 ++
 6 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c
index 80acc1e19387..77f2ed5a76a2 100644
--- a/fs/ext2/balloc.c
+++ b/fs/ext2/balloc.c
@@ -1236,6 +1236,11 @@ ext2_fsblk_t ext2_new_blocks(struct inode *inode, ext2_fsblk_t goal,
 	*errp = -ENOSPC;
 	sb = inode->i_sb;
 
+	if (sb_emergency_ro(sb)) {
+		*errp = -EROFS;
+		return 0;
+	}
+
 	/*
 	 * Check quota for allocation of this block.
 	 */
diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index e17bbc7598c1..c2c9fc2d0b27 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -445,6 +445,9 @@ static int ext2_handle_dirsync(struct inode *dir)
 	err = filemap_write_and_wait(dir->i_mapping);
 	if (!err)
 		err = sync_inode_metadata(dir, 1);
+	/* The change is done, an error made us read-only in the meantime. */
+	if (err == -EROFS && sb_emergency_ro(dir->i_sb))
+		err = 0;
 	return err;
 }
 
diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
index bf21b57cf98c..2741ecd2b94f 100644
--- a/fs/ext2/ialloc.c
+++ b/fs/ext2/ialloc.c
@@ -433,6 +433,8 @@ struct inode *ext2_new_inode(struct inode *dir, umode_t mode,
 	int err;
 
 	sb = dir->i_sb;
+	if (sb_emergency_ro(sb))
+		return ERR_PTR(-EROFS);
 	inode = new_inode(sb);
 	if (!inode)
 		return ERR_PTR(-ENOMEM);
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 12ac4cfe1500..5c4831f8f45d 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -71,7 +71,9 @@ void ext2_evict_inode(struct inode * inode)
 	struct ext2_block_alloc_info *rsv;
 	int want_delete = 0;
 
-	if (!inode->i_nlink && !is_bad_inode(inode)) {
+	/* After an error made us read-only the inode is left to e2fsck. */
+	if (!inode->i_nlink && !is_bad_inode(inode) &&
+	    !sb_emergency_ro(inode->i_sb)) {
 		want_delete = 1;
 		dquot_initialize(inode);
 	} else {
@@ -1481,10 +1483,14 @@ int ext2_write_inode(struct inode *inode, struct writeback_control *wbc)
 	uid_t uid = i_uid_read(inode);
 	gid_t gid = i_gid_read(inode);
 	struct buffer_head * bh;
-	struct ext2_inode * raw_inode = ext2_get_inode(sb, ino, &bh);
+	struct ext2_inode *raw_inode;
 	int n;
 	int err = 0;
 
+	if (sb_emergency_ro(sb))
+		return -EROFS;
+
+	raw_inode = ext2_get_inode(sb, ino, &bh);
 	if (IS_ERR(raw_inode))
  		return -EIO;
 
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index fb217ef2d69f..804a7224066d 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -53,7 +53,7 @@ void ext2_error(struct super_block *sb, const char *function,
 	struct ext2_sb_info *sbi = EXT2_SB(sb);
 	struct ext2_super_block *es = sbi->s_es;
 
-	if (!sb_rdonly(sb)) {
+	if (!sb_rdonly(sb) && !sb_emergency_ro(sb)) {
 		spin_lock(&sbi->s_lock);
 		sbi->s_mount_state |= EXT2_ERROR_FS;
 		es->s_state |= cpu_to_le16(EXT2_ERROR_FS);
@@ -73,11 +73,11 @@ void ext2_error(struct super_block *sb, const char *function,
 
 	if (test_opt(sb, ERRORS_PANIC))
 		panic("EXT2-fs: panic from previous error\n");
-	if (!sb_rdonly(sb) && test_opt(sb, ERRORS_RO)) {
+	/* SB_RDONLY needs s_umount, see sb_set_emergency_ro(). */
+	if (!sb_rdonly(sb) && test_opt(sb, ERRORS_RO) &&
+	    sb_set_emergency_ro(sb))
 		ext2_msg(sb, KERN_CRIT,
 			     "error: remounting filesystem read-only");
-		sb->s_flags |= SB_RDONLY;
-	}
 }
 
 static void ext2_msg_fc(struct fs_context *fc, const char *prefix,
@@ -179,7 +179,7 @@ static void ext2_put_super (struct super_block * sb)
 	ext2_xattr_destroy_cache(sbi->s_ea_block_cache);
 	sbi->s_ea_block_cache = NULL;
 
-	if (!sb_rdonly(sb)) {
+	if (!sb_rdonly(sb) && !sb_emergency_ro(sb)) {
 		struct ext2_super_block *es = sbi->s_es;
 
 		spin_lock(&sbi->s_lock);
@@ -1287,6 +1287,10 @@ static int ext2_sync_fs(struct super_block *sb, int wait)
 	struct ext2_sb_info *sbi = EXT2_SB(sb);
 	struct ext2_super_block *es = EXT2_SB(sb)->s_es;
 
+	/* Nothing may be written after an error made us read-only. */
+	if (sb_emergency_ro(sb))
+		return 0;
+
 	/*
 	 * Write quota structures to quota file, sync_blockdev() will write
 	 * them to disk later
@@ -1307,6 +1311,9 @@ static int ext2_freeze(struct super_block *sb)
 {
 	struct ext2_sb_info *sbi = EXT2_SB(sb);
 
+	if (sb_emergency_ro(sb))
+		return 0;
+
 	/*
 	 * Open but unlinked files present? Keep EXT2_VALID_FS flag cleared
 	 * because we have unattached inodes and thus filesystem is not fully
@@ -1349,7 +1356,9 @@ static int ext2_reconfigure(struct fs_context *fc)
 	int flags = fc->sb_flags;
 	int err;
 
-	sync_filesystem(sb);
+	/* Nothing may be written after an error made us read-only. */
+	if (!sb_emergency_ro(sb))
+		sync_filesystem(sb);
 
 	new_opts.s_mount_opt = ctx->vals_s_mount_opt;
 	new_opts.s_resuid = ctx->s_resuid;
@@ -1360,6 +1369,9 @@ static int ext2_reconfigure(struct fs_context *fc)
 	if ((bool)(flags & SB_RDONLY) == sb_rdonly(sb))
 		goto out_set;
 	if (flags & SB_RDONLY) {
+		/* After an error nothing may be written, just go read-only. */
+		if (sb_emergency_ro(sb))
+			goto out_set;
 		if (le16_to_cpu(es->s_state) & EXT2_VALID_FS ||
 		    !(sbi->s_mount_state & EXT2_VALID_FS))
 			goto out_set;
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 8f608930a48c..12dfffff189c 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -429,6 +429,8 @@ ext2_xattr_set(struct inode *inode, int name_index, const char *name,
 	ea_idebug(inode, "name=%d.%s, value=%p, value_len=%ld",
 		  name_index, name, value, (long)value_len);
 
+	if (sb_emergency_ro(sb))
+		return -EROFS;
 	if (value == NULL)
 		value_len = 0;
 	if (name == NULL)

-- 
2.53.0