All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josh Triplett <josh@joshtriplett.org>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Theodore Ts'o <tytso@mit.edu>,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	Jan Kara <jack@suse.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-ext4@vger.kernel.org
Subject: ext4 regression in v5.9-rc2 from e7bfb5c9bb3d on ro fs with overlapped bitmaps
Date: Mon, 5 Oct 2020 01:14:54 -0700	[thread overview]
Message-ID: <20201005081454.GA493107@localhost> (raw)
In-Reply-To: <CAHk-=wj-H5BYCU_kKiOK=B9sN3BtRzL4pnne2AJPyf54nQ+d=w@mail.gmail.com>

Ran into an ext4 regression when testing upgrades to 5.9-rc kernels:

Commit e7bfb5c9bb3d ("ext4: handle add_system_zone() failure in
ext4_setup_system_zone()") breaks mounting of read-only ext4 filesystems
with intentionally overlapping bitmap blocks.

On an always-read-only filesystem explicitly marked with
EXT4_FEATURE_RO_COMPAT_SHARED_BLOCKS, prior to that commit, it's safe to
point all the block and inode bitmaps to a single block of all 1s,
because a read-only filesystem will never allocate or free any blocks or
inodes.

However, after that commit, the block validity check rejects such
filesystems with -EUCLEAN and "failed to initialize system zone (-117)".
This causes systems that previously worked correctly to fail when
upgrading to v5.9-rc2 or later.

This was obviously a bugfix, and I'm not suggesting that it should be
reverted; it looks like this effectively worked by accident before,
because the block_validity check wasn't fully functional. However, this
does break real systems, and I'd like to get some kind of regression fix
in before 5.9 final if possible. I think it would suffice to make
block_validity default to false if and only if
EXT4_FEATURE_RO_COMPAT_SHARED_BLOCKS is set.

Does that seem like a reasonable fix?

Here's a quick sketch of a patch, which I've tested and confirmed to
work:

----- 8< -----
Subject: [PATCH] Fix ext4 regression in v5.9-rc2 on ro fs with overlapped bitmaps

Commit e7bfb5c9bb3d ("ext4: handle add_system_zone() failure in
ext4_setup_system_zone()") breaks mounting of read-only ext4 filesystems
with intentionally overlapping bitmap blocks.

On an always-read-only filesystem explicitly marked with
EXT4_FEATURE_RO_COMPAT_SHARED_BLOCKS, prior to that commit, it's safe to
point all the block and inode bitmaps to a single block of all 1s,
because a read-only filesystem will never allocate or free any blocks or
inodes.

However, after that commit, the block validity check rejects such
filesystems with -EUCLEAN and "failed to initialize system zone (-117)".
This causes systems that previously worked correctly to fail when
upgrading to v5.9-rc2 or later.

Fix this by defaulting block_validity to off when
EXT4_FEATURE_RO_COMPAT_SHARED_BLOCKS is set.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Fixes: e7bfb5c9bb3d ("ext4: handle add_system_zone() failure in ext4_setup_system_zone()")
---
 fs/ext4/ext4.h  | 2 ++
 fs/ext4/super.c | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 523e00d7b392..7874028fa864 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1834,6 +1834,7 @@ static inline bool ext4_verity_in_progress(struct inode *inode)
 #define EXT4_FEATURE_RO_COMPAT_METADATA_CSUM	0x0400
 #define EXT4_FEATURE_RO_COMPAT_READONLY		0x1000
 #define EXT4_FEATURE_RO_COMPAT_PROJECT		0x2000
+#define EXT4_FEATURE_RO_COMPAT_SHARED_BLOCKS	0x4000
 #define EXT4_FEATURE_RO_COMPAT_VERITY		0x8000
 
 #define EXT4_FEATURE_INCOMPAT_COMPRESSION	0x0001
@@ -1930,6 +1931,7 @@ EXT4_FEATURE_RO_COMPAT_FUNCS(bigalloc,		BIGALLOC)
 EXT4_FEATURE_RO_COMPAT_FUNCS(metadata_csum,	METADATA_CSUM)
 EXT4_FEATURE_RO_COMPAT_FUNCS(readonly,		READONLY)
 EXT4_FEATURE_RO_COMPAT_FUNCS(project,		PROJECT)
+EXT4_FEATURE_RO_COMPAT_FUNCS(shared_blocks,	SHARED_BLOCKS)
 EXT4_FEATURE_RO_COMPAT_FUNCS(verity,		VERITY)
 
 EXT4_FEATURE_INCOMPAT_FUNCS(compression,	COMPRESSION)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ea425b49b345..f57a7e966e44 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3954,7 +3954,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	else
 		set_opt(sb, ERRORS_RO);
 	/* block_validity enabled by default; disable with noblock_validity */
-	set_opt(sb, BLOCK_VALIDITY);
+	if (!ext4_has_feature_shared_blocks(sb))
+		set_opt(sb, BLOCK_VALIDITY);
 	if (def_mount_opts & EXT4_DEFM_DISCARD)
 		set_opt(sb, DISCARD);
 

  reply	other threads:[~2020-10-05  8:22 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-04 23:17 Linux 5.9-rc8 Linus Torvalds
2020-10-05  8:14 ` Josh Triplett [this message]
2020-10-05  9:46   ` ext4 regression in v5.9-rc2 from e7bfb5c9bb3d on ro fs with overlapped bitmaps Jan Kara
2020-10-05 10:16     ` Josh Triplett
2020-10-05 16:19       ` Jan Kara
2020-10-05 16:20   ` Jan Kara
2020-10-05 17:36   ` Darrick J. Wong
2020-10-06  0:04     ` Theodore Y. Ts'o
2020-10-06  0:32     ` Josh Triplett
2020-10-06  2:51       ` Darrick J. Wong
2020-10-06  3:18         ` Theodore Y. Ts'o
2020-10-06  5:03           ` Josh Triplett
2020-10-06  6:03             ` Josh Triplett
2020-10-06 13:35             ` Theodore Y. Ts'o
2020-10-07  8:03               ` Josh Triplett
2020-10-07 14:32                 ` Theodore Y. Ts'o
2020-10-07 20:14                   ` Josh Triplett
2020-10-08  2:10                     ` Theodore Y. Ts'o
2020-10-08 17:54                       ` Darrick J. Wong
2020-10-08 22:38                         ` Josh Triplett
2020-10-09  2:54                           ` Darrick J. Wong
2020-10-09 19:08                             ` Josh Triplett
2020-10-08 22:22                       ` Josh Triplett
2020-10-09 14:37                         ` Theodore Y. Ts'o
2020-10-09 20:30                           ` Josh Triplett
2021-01-10 18:41                           ` Malicious fs images was " Pavel Machek
2021-01-11 18:51                             ` Darrick J. Wong
2021-01-11 19:39                               ` Eric Biggers
2021-01-12 21:43                             ` Theodore Ts'o
2021-01-12 22:28                               ` Pavel Machek
2021-01-13  5:09                                 ` Theodore Ts'o
2020-10-08  2:57                     ` Andreas Dilger
2020-10-08 19:12                       ` Josh Triplett
2020-10-08 19:25                         ` Andreas Dilger
2020-10-08 22:28                           ` Josh Triplett

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=20201005081454.GA493107@localhost \
    --to=josh@joshtriplett.org \
    --cc=adilger.kernel@dilger.ca \
    --cc=jack@suse.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.