Re: [PATCH v3 12/33] exfat: use the generic emergency read-only state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] exfat: race with emergency read-only transition in
  exfat_force_shutdown()
--

commit 55555fc2a6a36c47e1e61fcbe8c22b69f7d5a5f8
Author: Christian Brauner <brauner@kernel.org>

exfat: use the generic emergency read-only state

This commit replaces EXFAT_FLAGS_EMERGENCY_RO with the VFS generic
SB_STATE_EMERGENCY_RO state and skips freezing in exfat_force_shutdown()
when the superblock is already in the emergency read-only state.

> diff --git a/fs/exfat/super.c b/fs/exfat/super.c
> index feeda41a80741..e3587ed270245 100644
> --- a/fs/exfat/super.c
> +++ b/fs/exfat/super.c

[ ... ]

> @@ -176,6 +176,11 @@ int exfat_force_shutdown(struct super_block *sb, u32 flags)
>  	if (exfat_forced_shutdown(sb))
>  		return 0;
>  
> +	/* After an error there is nothing to sync and freezing fails. */
> +	if ((flags == EXFAT_GOING_DOWN_DEFAULT ||
> +	     flags == EXFAT_GOING_DOWN_FULLSYNC) && sb_emergency_ro(sb))
> +		flags = EXFAT_GOING_DOWN_NOSYNC;
> +
>  	switch (flags) {
>  	case EXFAT_GOING_DOWN_DEFAULT:
>  	case EXFAT_GOING_DOWN_FULLSYNC:

[Severity: Medium]
Can exfat_force_shutdown() abort without setting EXFAT_FLAGS_SHUTDOWN if a
filesystem error sets SB_STATE_EMERGENCY_RO while bdev_freeze() is running?

CPU0                                    CPU1
exfat_force_shutdown()
  sb_emergency_ro(sb) -> false
  bdev_freeze(sb->s_bdev)
    ...
    freeze_super(sb, ...)
      sb_wait_write(sb, ...) /
      sync_filesystem(sb)
                                        __exfat_fs_error(sb, ...)
                                          sb_set_emergency_ro(sb)
      if (!ret && sb_emergency_ro(sb))
        ret = -EROFS;
  if (ret)
    return ret;  <-- aborts without shutting down

Because freeze_super() checks sb_emergency_ro(sb) both on entry and after
sync_filesystem(sb), an in-flight writer or writeback error during the
freeze makes bdev_freeze() return -EROFS.  exfat_force_shutdown() then
returns -EROFS without setting EXFAT_FLAGS_SHUTDOWN or clearing
opts->discard.  Ignoring -EROFS from bdev_freeze() when sb_emergency_ro(sb)
is set would still complete the shutdown.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-work-fs-sb_rdonly-v3-0-0be92bb73709@kernel.org?part=12