Re: [PATCH RFC] fs: fix superblock freeze rollback on sync_blockdev failure
Uladzislau Zhauniarovich <[email protected]>
| Newsgroups | dev.linux.lists.syzbot |
|---|---|
| Message-ID | <[email protected]> |
The core idea is right — exFAT must not set SB_RDONLY on its own —
but this version has two problems and the fix should stay inside exfat.
1) Don't reuse EXFAT_FLAGS_SHUTDOWN.
Since commit 47e35366bc6f ("exfat: fix missing shutdown check"), the read
paths ->read_iter, ->splice_read, ->mmap and ->file_open all test
exfat_forced_shutdown() and return -EIO. Setting EXFAT_FLAGS_SHUTDOWN from
the error handler therefore breaks *reads* with -EIO and turns the default
errors=remount-ro into a full shutdown, which is a regression. It also
re-logs "Filesystem has been set read-only" on every subsequent error.
Please track the post-error read-only state in a *separate* superblock flag
(e.g. EXFAT_FLAGS_ERROR_RO) and fail only *modifying* operations with
-EROFS, exactly like ext4's EXT4_FLAGS_EMERGENCY_RO / ext4_emergency_state()
(commit d3476f3dad4a "ext4: don't set SB_RDONLY after filesystem errors")
and f2fs (commit 930c6ab93492). Concretely:
- add a new flag EXFAT_FLAGS_ERROR_RO;
- in __exfat_fs_error(), for EXFAT_ERRORS_RO, test_and_set that bit
instead of "sb->s_flags |= SB_RDONLY" (log once);
- add a small helper (returning -EIO on shutdown, -EROFS on error-RO) and
call it at every modifying entry point only: ->write_iter,
->page_mkwrite, ->setattr, ->fallocate, and the namei ops create,
unlink, mkdir, rmdir, rename;
- leave ->read_iter, ->splice_read, ->mmap, ->file_open, ->fsync and
writeback untouched, so reads and flushing of already-dirty data keep
working just as they did with SB_RDONLY;
- clear the flag on a read-write remount in exfat_reconfigure(), so the
filesystem can be made writable again by remounting, as documented.
2) Drop the fs/super.c hunk.
It does not fix this warning. freeze_super() holds an s_active reference for
the freeze, so a superblock left frozen is never torn down —
destroy_super_work()
does not run and the rcu_sync_dtor() splat cannot originate there. The
actual
race is entirely inside exfat: __exfat_fs_error() flips SB_RDONLY without
sb->s_umount between freeze_super() (which took the s_writers
semaphores) and
thaw_super_locked() (which re-reads sb_rdonly() and then skips
sb_freeze_unlock()). Fixing exfat alone is sufficient; please remove the
fs/super.c change and keep this exfat-only.
3) Fixes tag.
Use the cause-bisection commit:
Fixes: f761fcdd289d ("exfat: Implement sops->shutdown and ioctl")
not 49ef8832fb1a.
On 20/07/2026 16:25, syzbot wrote:
> If sync_blockdev() fails during fs_bdev_freeze(), the superblock is left in
> a frozen state, but the block device is not. When the filesystem is later
> unmounted and destroyed, the s_writers.rw_sem per-CPU rw-semaphores are
> freed while still held for write, triggering a warning in rcu_sync_dtor():
>
> READ_ONCE(rsp->gp_state) == GP_PASSED
> WARNING: kernel/rcu/sync.c:177 at rcu_sync_dtor+0xcd/0x180
> kernel/rcu/sync.c:177, CPU#0: kworker/0:2/5024
> Call Trace:
> <TASK>
> percpu_free_rwsem+0x43/0x80 kernel/locking/percpu-rwsem.c:42
> destroy_super_work+0x217/0x310 fs/super.c:284
> process_one_work kernel/workqueue.c:3322 [inline]
> process_scheduled_works+0xa8e/0x14e0 kernel/workqueue.c:3405
> worker_thread+0xa47/0xfb0 kernel/workqueue.c:3486
> kthread+0x388/0x470 kernel/kthread.c:436
> ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
> ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
> </TASK>
>
> This happens because of two issues. First, in fs_bdev_freeze(), if
> freeze_super() succeeds but sync_blockdev() fails, thaw_super() is not
> called to roll back the freeze. Second, in exfat, an I/O error during
> sync_blockdev() triggers the error handler (__exfat_fs_error()), which
> directly sets SB_RDONLY without holding the s_umount semaphore. If
> thaw_super() is called after this, it sees the filesystem as read-only and
> skips releasing the freeze semaphores.
>
> Fix this by adding the missing thaw_super() rollback in fs_bdev_freeze()
> when sync_blockdev() fails. Additionally, modify the exfat error handler to
> set EXFAT_FLAGS_SHUTDOWN instead of directly setting SB_RDONLY, which
> prevents confusing the VFS freeze logic and allows thaw_super() to
> correctly release the semaphores.
>
> Fixes: 49ef8832fb1a ("bdev: implement freeze and thaw holder operations")
> Assisted-by: Gemini:gemini-3.5-flash Gemini:gemini-3.1-pro-preview syzbot
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=823cd0d24881f21ab9f1
> Link: https://syzkaller.appspot.com/ai_job?id=431fcee8-0777-4619-a114-4a86af680864
> To: "Christian Brauner" <[email protected]>
> To: "Namjae Jeon" <[email protected]>
> To: <[email protected]>
> To: "Sungjong Seo" <[email protected]>
> To: "Alexander Viro" <[email protected]>
> Cc: "Jan Kara" <[email protected]>
> Cc: <[email protected]>
> Cc: "Yuezhang Mo" <[email protected]>
>
> ---
> diff --git a/fs/exfat/misc.c b/fs/exfat/misc.c
> index 6f11a96a4..dbb3b2907 100644
> --- a/fs/exfat/misc.c
> +++ b/fs/exfat/misc.c
> @@ -41,7 +41,14 @@ void __exfat_fs_error(struct super_block *sb, int report, const char *fmt, ...)
> panic("exFAT-fs (%s): fs panic from previous error\n",
> sb->s_id);
> } else if (opts->errors == EXFAT_ERRORS_RO && !sb_rdonly(sb)) {
> - sb->s_flags |= SB_RDONLY;
> + set_bit(EXFAT_FLAGS_SHUTDOWN, &EXFAT_SB(sb)->s_exfat_flags);
> + /*
> + * We have already set EXFAT_FLAGS_SHUTDOWN flag to stop all updates
> + * to filesystem, so it doesn't need to set SB_RDONLY flag here
> + * because the flag should be set covered w/ sb->s_umount semaphore
> + * via remount procedure, otherwise, it will confuse code like
> + * freeze_super() which will lead to deadlocks and other problems.
> + */
> exfat_err(sb, "Filesystem has been set read-only");
> }
> }
> diff --git a/fs/super.c b/fs/super.c
> index a8fd61136..909d8b9ab 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1481,8 +1481,23 @@ static int fs_bdev_freeze(struct block_device *bdev)
> else
> error = freeze_super(sb,
> FREEZE_MAY_NEST | FREEZE_HOLDER_USERSPACE, NULL);
> - if (!error)
> + if (!error) {
> error = sync_blockdev(bdev);
> + if (error) {
> + if (sb->s_op->thaw_super)
> + (void)sb->s_op->thaw_super(
> + sb,
> + FREEZE_MAY_NEST |
> + FREEZE_HOLDER_USERSPACE,
> + NULL);
> + else
> + (void)thaw_super(
> + sb,
> + FREEZE_MAY_NEST |
> + FREEZE_HOLDER_USERSPACE,
> + NULL);
> + }
> + }
> deactivate_super(sb);
> return error;
> }
>
>
> base-commit: 1590cf0329716306e948a8fc29f1d3ee87d3989f