Linux XFS filesystem development
 help / color / mirror / Atom feed
* [PATCH RFC 00/28] fs: don't set SB_RDONLY after filesystem errors
@ 2026-10-12 10:20 Christian Brauner
  2026-10-12 10:20 ` [PATCH RFC 01/28] ecryptfs: don't leak the inode when a lookup can't open the lower file Christian Brauner
                   ` (28 more replies)
  0 siblings, 29 replies; 37+ messages in thread
From: Christian Brauner @ 2026-10-12 10:20 UTC (permalink / raw)
  To: Jan Kara
  Cc: Al Viro, ecryptfs, linux-ext4, jfs-discussion, linux-nilfs, exfat,
	linux-fsdevel, ntfs, ocfs2-devel, linux-btrfs, gfs2, linux-efi,
	linux-mtd, linux-xfs, Christian Brauner (Amutable), stable,
	Karl Mehltretter

A lot of filesystems handle errors by setting SB_RDONLY on the
superblock. They do that from whatever context found the error and
without holding s_umount. That's never been okay. SB_RDONLY is supposed
to only change under s_umount through a proper remount.

And it breaks freezing. freeze_super() and thaw_super() both look at
sb_rdonly() to figure out whether the s_writers rwsems need to be taken
or released. So if an error sets SB_RDONLY while the filesystem is
frozen thaw_super() takes the read-only shortcut, leaves the rwsems
write-locked and marks the superblock unfrozen. Every writer after that
hangs in sb_start_write() and umount warns when it frees the locked
rwsems. jfs even oopses on umount. And the unlocked update of
sb->s_flags races with everyone else who writes it.

Karl noticed and proposed teaching thaw_super() to remember whether the
freeze took the rwsems (cf. [1]). But a filesystem changing SB_RDONLY
behind the VFS' back causes more problems than just confused freezing.
Jan preferred fixing the filesystems instead (cf. [2]) and I agree. ext4
and f2fs already stopped doing it. So this converts all the others to
what ext4 does:

- the error is recorded in filesystem private state and the filesystem
  refuses modifications with EROFS itself
- the superblock stays read-write and "emergency_ro" is shown in the
  mount options ("withdrawn" for gfs2, "ro_ops" for efivarfs)
- a remount to read-write fails with EROFS until umount
- a remount to read-only works so emergency remount (sysrq u) doesn't
  WARN, and it doesn't mark the filesystem clean

That's ext2, jfs, nilfs2, fat, exfat, hfs, affs, ufs, hpfs, ntfs, ocfs2,
zonefs, btrfs, gfs2, efivarfs and ubifs. For gfs2 it's the withdraw and
for efivarfs it's the notifier that flips SB_RDONLY when the efivar ops
switch between ops that can and can't set variables. ubifs can't be
frozen but it flips SB_RDONLY from any context all the same. hfsplus
only did it when it created the attributes tree, where it can't trigger,
so that check is simply limited to mount.

fat, exfat, hpfs, ntfs and ubifs keep setting SB_RDONLY for errors while
the filesystem is still being mounted. Until SB_BORN the mounter holds
s_umount and nothing can freeze the superblock so that's fine.

A few patches make the read-only remount work in the first place. exfat
and ocfs2 didn't inherit their mount options on remount so emergency
remount, which passes none, could fail on them. And gfs2 has a BUG_ON()
when it's remounted read-only while a withdraw is still pending that
this makes easier to hit. Those come before the conversion of the
respective filesystem. ext4 and xfs have the same problem with their own
error states. ext4 refused to go read-only in its emergency state at all
and a shut down xfs couldn't be remounted read-only either. So sysrq u
WARNed for every ext4 that hit an error and for every shut down xfs.
Both are fixed at the end of the series.

ecryptfs opens its lower files read-write unless the lower superblock is
read-only. On top of one of these filesystems every lookup of a regular
file would fail with EROFS after an error and leak the inode. umount
then reports busy inodes. ext4 already behaves like that and so does a
read-only bind mount underneath. The first two patches make ecryptfs
fall back to a read-only lower file and plug the leak.

There are user-visible changes but I think they're all fine. The big one
is that "mount -o remount,rw" doesn't make the filesystem writable again
after an error anymore. That's how ext4 behaves since
commit d3476f3dad4a ("ext4: don't set SB_RDONLY after filesystem
errors"). statfs() doesn't report ST_RDONLY after an error and a second
read-only mount of the device fails with EBUSY. Writes through file
descriptors opened before the error fail with EROFS now where they used
to succeed. ext2, fat, hfs, affs, ufs and ubifs fail fsync() with EROFS
after an error. Writeback drops what they can't write anymore so fsync()
would report success otherwise. And remount helpers that pass the shown
options back, busybox mount for example, trip over "emergency_ro".
That's already the case on ext4.

Every patch has a reproducer in the commit message. I ran all of them
in a VM against the unpatched and the patched kernel, the latter with
KASAN and kmemleak. There are selftests for ext2, jfs, nilfs2, gfs2 and
the ext4 and xfs remount changes.

This depends on three fixes that aren't upstream yet:

  super: don't put the remount context under s_umount
  super: keep the read-only state if a reconfiguration doesn't change it
  nilfs2: don't reconfigure a frozen superblock on mount

The first one matters most. This series makes errored filesystems
reachable for sysrq u and without it a concurrent umount deadlocks on
s_umount. The second one makes fc->sb_flags carry the current read-only
state for fspick() reconfigurations, which ->reconfigure() of fat, affs,
hfs, ufs, hpfs and btrfs rely on. The third one closes the last way
nilfs2 changes SB_RDONLY on a frozen superblock, through a mount that
reconfigures it.

With this no filesystem changes SB_RDONLY outside of mount and remount
anymore.

[1]: https://lore.kernel.org/r/20260903060855.4610-1-kmehltretter@gmail.com
[2]: https://lore.kernel.org/r/hyhuckyohsabx2suvq3z7uy3pbgmihva6wgzxr3op7pzcrlu33@p7zzlgp2ehjf

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Christian Brauner (28):
      ecryptfs: don't leak the inode when a lookup can't open the lower file
      ecryptfs: fall back to a read-only lower file
      ext2: don't set SB_RDONLY after filesystem errors
      jfs: don't set SB_RDONLY after filesystem errors
      nilfs2: don't set SB_RDONLY after filesystem errors
      fat: don't set SB_RDONLY after filesystem errors
      exfat: keep the current mount options on remount
      exfat: don't set SB_RDONLY after filesystem errors
      hfs: don't set SB_RDONLY after MDB write errors
      hfsplus: don't set SB_RDONLY when creating the attributes tree
      affs: don't set SB_RDONLY after filesystem errors
      ufs: don't set SB_RDONLY after filesystem errors
      hpfs: don't set SB_RDONLY after filesystem errors
      ntfs: don't set SB_RDONLY after filesystem errors
      ocfs2: keep options that can't change on remount
      ocfs2: don't set SB_RDONLY after filesystem errors
      zonefs: don't set SB_RDONLY after I/O errors
      btrfs: check BTRFS_FS_STATE_RO instead of SB_RDONLY at runtime
      btrfs: don't set SB_RDONLY after filesystem errors
      gfs2: wait for a pending withdraw before remounting read-only
      gfs2: don't set SB_RDONLY on withdraw
      efivarfs: stop using SB_RDONLY for read-only efivar ops
      ubifs: don't set SB_RDONLY after errors
      selftests/filesystems: test filesystem errors while frozen
      ext4: allow remounting read-only in the emergency state
      selftests/filesystems: test ext4 remounts in the emergency state
      xfs: allow remounting read-only after a shutdown
      selftests/filesystems: test xfs remounts after a shutdown

 Documentation/filesystems/zonefs.rst               |   5 +-
 fs/affs/affs.h                                     |   9 +
 fs/affs/amigaffs.c                                 |  10 +-
 fs/affs/file.c                                     |  16 +-
 fs/affs/inode.c                                    |   5 +
 fs/affs/namei.c                                    |  18 +
 fs/affs/super.c                                    |  24 +-
 fs/btrfs/acl.c                                     |   3 +-
 fs/btrfs/bio.c                                     |   2 +-
 fs/btrfs/block-group.c                             |   2 +-
 fs/btrfs/dev-replace.c                             |   2 +-
 fs/btrfs/discard.c                                 |   2 +-
 fs/btrfs/disk-io.c                                 |   4 +-
 fs/btrfs/fs.h                                      |  14 +
 fs/btrfs/inode.c                                   |   8 +-
 fs/btrfs/ioctl.c                                   |   8 +-
 fs/btrfs/messages.c                                |  21 +-
 fs/btrfs/qgroup.c                                  |   4 +-
 fs/btrfs/reflink.c                                 |   3 +-
 fs/btrfs/super.c                                   |  45 +-
 fs/btrfs/sysfs.c                                   |   6 +-
 fs/btrfs/tree-checker.c                            |   2 +-
 fs/btrfs/volumes.c                                 |   4 +-
 fs/btrfs/xattr.c                                   |   8 +-
 fs/ecryptfs/inode.c                                |   5 +-
 fs/ecryptfs/kthread.c                              |  19 +-
 fs/efivarfs/file.c                                 |   6 +
 fs/efivarfs/inode.c                                |  15 +
 fs/efivarfs/internal.h                             |  14 +-
 fs/efivarfs/super.c                                |  31 +-
 fs/exfat/exfat_fs.h                                |  19 +
 fs/exfat/file.c                                    |  54 +-
 fs/exfat/inode.c                                   |  11 +-
 fs/exfat/misc.c                                    |  24 +-
 fs/exfat/namei.c                                   |  27 +-
 fs/exfat/super.c                                   |  49 +-
 fs/ext2/balloc.c                                   |   5 +
 fs/ext2/dir.c                                      |   5 +-
 fs/ext2/ext2.h                                     |  14 +
 fs/ext2/file.c                                     |  16 +-
 fs/ext2/ialloc.c                                   |   2 +
 fs/ext2/inode.c                                    |  13 +-
 fs/ext2/ioctl.c                                    |   7 +
 fs/ext2/namei.c                                    |  10 +
 fs/ext2/super.c                                    |  62 ++-
 fs/ext2/xattr.c                                    |   2 +
 fs/ext4/super.c                                    |  20 +-
 fs/fat/dir.c                                       |   3 +
 fs/fat/fat.h                                       |  13 +
 fs/fat/fatent.c                                    |  11 +-
 fs/fat/file.c                                      |  19 +
 fs/fat/inode.c                                     |  26 +-
 fs/fat/misc.c                                      |  29 +-
 fs/fat/namei_msdos.c                               |   9 +
 fs/fat/namei_vfat.c                                |   9 +
 fs/gfs2/ops_fstype.c                               |   6 +-
 fs/gfs2/quota.c                                    |   2 +-
 fs/gfs2/super.c                                    |  41 +-
 fs/gfs2/util.c                                     |   8 +-
 fs/hfs/attr.c                                      |   3 +
 fs/hfs/dir.c                                       |  12 +
 fs/hfs/extent.c                                    |   5 +
 fs/hfs/hfs_fs.h                                    |  11 +
 fs/hfs/inode.c                                     |  30 +
 fs/hfs/mdb.c                                       |  18 +-
 fs/hfs/super.c                                     |   7 +-
 fs/hfsplus/btree.c                                 |   7 +-
 fs/hpfs/alloc.c                                    |   4 +-
 fs/hpfs/dir.c                                      |   3 +-
 fs/hpfs/file.c                                     |  17 +-
 fs/hpfs/hpfs_fn.h                                  |   9 +
 fs/hpfs/inode.c                                    |   2 +
 fs/hpfs/map.c                                      |   3 +-
 fs/hpfs/namei.c                                    |  14 +
 fs/hpfs/super.c                                    |  22 +-
 fs/jfs/acl.c                                       |   3 +
 fs/jfs/file.c                                      |  16 +-
 fs/jfs/inode.c                                     |   6 +-
 fs/jfs/ioctl.c                                     |   7 +
 fs/jfs/jfs_dtree.c                                 |   3 +-
 fs/jfs/jfs_extent.c                                |  10 +
 fs/jfs/jfs_incore.h                                |  12 +
 fs/jfs/jfs_inode.c                                 |   3 +
 fs/jfs/namei.c                                     |  12 +
 fs/jfs/super.c                                     |  69 ++-
 fs/jfs/xattr.c                                     |   3 +
 fs/nilfs2/file.c                                   |  15 +-
 fs/nilfs2/inode.c                                  |  13 +-
 fs/nilfs2/ioctl.c                                  |  36 +-
 fs/nilfs2/mdt.c                                    |   2 +-
 fs/nilfs2/namei.c                                  |   1 +
 fs/nilfs2/page.c                                   |   2 +-
 fs/nilfs2/segment.c                                |  13 +-
 fs/nilfs2/sufile.h                                 |   2 +-
 fs/nilfs2/super.c                                  |  35 +-
 fs/nilfs2/the_nilfs.h                              |   8 +
 fs/ntfs/aops.c                                     |   5 +-
 fs/ntfs/dir.c                                      |   5 +-
 fs/ntfs/ea.c                                       |  10 +-
 fs/ntfs/file.c                                     |  51 +-
 fs/ntfs/inode.c                                    |  22 +-
 fs/ntfs/iomap.c                                    |   6 +-
 fs/ntfs/mft.c                                      |  10 +-
 fs/ntfs/namei.c                                    |  40 +-
 fs/ntfs/super.c                                    |  58 +-
 fs/ntfs/volume.h                                   |  22 +
 fs/ocfs2/acl.c                                     |   3 +
 fs/ocfs2/file.c                                    |   8 +
 fs/ocfs2/ioctl.c                                   |   3 +
 fs/ocfs2/mmap.c                                    |   3 +
 fs/ocfs2/namei.c                                   |  21 +
 fs/ocfs2/ocfs2.h                                   |  12 +
 fs/ocfs2/refcounttree.c                            |   3 +
 fs/ocfs2/super.c                                   |  77 ++-
 fs/ocfs2/xattr.c                                   |   8 +
 fs/ubifs/dir.c                                     |   1 +
 fs/ubifs/file.c                                    |  24 +
 fs/ubifs/io.c                                      |   7 +-
 fs/ubifs/ioctl.c                                   |   4 +
 fs/ubifs/super.c                                   |   8 +-
 fs/ubifs/ubifs.h                                   |   1 +
 fs/ubifs/xattr.c                                   |   6 +
 fs/ufs/cylinder.c                                  |  12 +-
 fs/ufs/dir.c                                       |   5 +-
 fs/ufs/file.c                                      |  28 +-
 fs/ufs/inode.c                                     |   5 +
 fs/ufs/namei.c                                     |  22 +
 fs/ufs/super.c                                     |  74 ++-
 fs/ufs/ufs.h                                       |  10 +
 fs/xfs/xfs_super.c                                 |  15 +-
 fs/zonefs/file.c                                   |  17 +-
 fs/zonefs/super.c                                  |  37 +-
 fs/zonefs/zonefs.h                                 |  16 +
 tools/testing/selftests/Makefile                   |   1 +
 .../selftests/filesystems/sb_rdonly/.gitignore     |   5 +
 .../selftests/filesystems/sb_rdonly/Makefile       |   7 +
 .../testing/selftests/filesystems/sb_rdonly/config |   7 +
 .../filesystems/sb_rdonly/emergency_ro_test.c      | 612 +++++++++++++++++++++
 .../sb_rdonly/ext4_emergency_remount_test.c        | 312 +++++++++++
 .../filesystems/sb_rdonly/gfs2_withdraw_test.c     | 330 +++++++++++
 .../selftests/filesystems/sb_rdonly/settings       |   1 +
 .../sb_rdonly/xfs_shutdown_remount_test.c          | 307 +++++++++++
 142 files changed, 3216 insertions(+), 294 deletions(-)
---
base-commit: a5ebb76233b79db01e82e061173cb73c6d2b5c6b
change-id: 20261011-work-fs-sb_rdonly-f93e626bd0c2


^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2026-10-13 19:39 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 10:20 [PATCH RFC 00/28] fs: don't set SB_RDONLY after filesystem errors Christian Brauner
2026-10-12 10:20 ` [PATCH RFC 01/28] ecryptfs: don't leak the inode when a lookup can't open the lower file Christian Brauner
2026-10-12 10:20 ` [PATCH RFC 02/28] ecryptfs: fall back to a read-only " Christian Brauner
2026-10-12 10:20 ` [PATCH RFC 03/28] ext2: don't set SB_RDONLY after filesystem errors Christian Brauner
2026-10-12 10:20 ` [PATCH RFC 04/28] jfs: " Christian Brauner
2026-10-12 10:20 ` [PATCH RFC 05/28] nilfs2: " Christian Brauner
2026-10-13 16:01   ` Ryusuke Konishi
2026-10-12 10:20 ` [PATCH RFC 06/28] fat: " Christian Brauner
2026-10-12 10:20 ` [PATCH RFC 07/28] exfat: keep the current mount options on remount Christian Brauner
2026-10-12 10:20 ` [PATCH RFC 08/28] exfat: don't set SB_RDONLY after filesystem errors Christian Brauner
2026-10-12 10:20 ` [PATCH RFC 09/28] hfs: don't set SB_RDONLY after MDB write errors Christian Brauner
2026-10-12 22:57   ` [Jfs-discussion] " Viacheslav Dubeyko
2026-10-12 10:20 ` [PATCH RFC 10/28] hfsplus: don't set SB_RDONLY when creating the attributes tree Christian Brauner
2026-10-12 22:34   ` [Jfs-discussion] " Viacheslav Dubeyko
2026-10-12 10:20 ` [PATCH RFC 11/28] affs: don't set SB_RDONLY after filesystem errors Christian Brauner
2026-10-12 10:20 ` [PATCH RFC 12/28] ufs: " Christian Brauner
2026-10-12 10:20 ` [PATCH RFC 13/28] hpfs: " Christian Brauner
2026-10-12 10:20 ` [PATCH RFC 14/28] ntfs: " Christian Brauner
2026-10-12 10:21 ` [PATCH RFC 15/28] ocfs2: keep options that can't change on remount Christian Brauner
2026-10-12 10:21 ` [PATCH RFC 16/28] ocfs2: don't set SB_RDONLY after filesystem errors Christian Brauner
2026-10-12 10:21 ` [PATCH RFC 17/28] zonefs: don't set SB_RDONLY after I/O errors Christian Brauner
2026-10-12 10:21 ` [PATCH RFC 18/28] btrfs: check BTRFS_FS_STATE_RO instead of SB_RDONLY at runtime Christian Brauner
2026-10-12 10:21 ` [PATCH RFC 19/28] btrfs: don't set SB_RDONLY after filesystem errors Christian Brauner
2026-10-12 10:21 ` [PATCH RFC 20/28] gfs2: wait for a pending withdraw before remounting read-only Christian Brauner
2026-10-12 10:21 ` [PATCH RFC 21/28] gfs2: don't set SB_RDONLY on withdraw Christian Brauner
2026-10-12 10:21 ` [PATCH RFC 22/28] efivarfs: stop using SB_RDONLY for read-only efivar ops Christian Brauner
2026-10-12 10:21 ` [PATCH RFC 23/28] ubifs: don't set SB_RDONLY after errors Christian Brauner
2026-10-12 10:21 ` [PATCH RFC 24/28] selftests/filesystems: test filesystem errors while frozen Christian Brauner
2026-10-12 10:21 ` [PATCH RFC 25/28] ext4: allow remounting read-only in the emergency state Christian Brauner
2026-10-12 16:38   ` Jan Kara
2026-10-12 10:21 ` [PATCH RFC 26/28] selftests/filesystems: test ext4 remounts " Christian Brauner
2026-10-12 10:21 ` [PATCH RFC 27/28] xfs: allow remounting read-only after a shutdown Christian Brauner
2026-10-13  7:38   ` Christoph Hellwig
2026-10-12 10:21 ` [PATCH RFC 28/28] selftests/filesystems: test xfs remounts " Christian Brauner
2026-10-13  7:40   ` Christoph Hellwig
2026-10-12 16:53 ` [PATCH RFC 00/28] fs: don't set SB_RDONLY after filesystem errors Jan Kara
2026-10-13 19:39   ` Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox