Re: [PATCH] super: remember whether freeze holds writer rwsems

[Jan Kara]

On Thu 03-09-26 08:08:55, Karl Mehltretter wrote:
> freeze_super() does not acquire the writer rwsems when the superblock is
> read-only. thaw_super_locked() currently decides whether to release them
> from the current SB_RDONLY flag. Filesystem error paths can change that
> flag between freeze and thaw without s_umount serialization.
> 
> If a writable freeze is followed by a forced read-only transition, thaw
> reports success but skips ->unfreeze_fs() and sb_freeze_unlock(). The
> superblock is marked unfrozen while all writer rwsems remain write-locked.
> Conversely, if a filesystem is read-only when frozen and SB_RDONLY is
> cleared before thaw, thaw can release rwsems that were never acquired.
> 
> Record whether a successful freeze acquired the writer rwsems and use that
> state during thaw instead of re-sampling SB_RDONLY.
> 
> Fixes: 8129ed29644b ("change sb_writers to use percpu_rw_semaphore")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@xxxxxxxxx>

This looks like one of these theoretical issues LLMs come up with. Frankly,
I just wouldn't bother. If the fs gets forcibly remounted RO due to
metadata errors, a frozen fs is likely the least of your worries.

								Honza

> ---
> Tested with a test-only KUnit case in x86_64 QEMU on linux-next
> 32b6ef9a5d0e (next-20260902). It freezes a writable ramfs, sets SB_RDONLY,
> thaws it, and calls sb_start_write_trylock(). The call fails without this
> patch and succeeds with it.
> 
> Backport note: before e0b62a4dee24 ("fs: add fs/super_types.h header"),
> struct sb_writers is in include/linux/fs.h.
> 
>  fs/super.c                     | 4 +++-
>  include/linux/fs/super_types.h | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index 9d40252135212..e8d75cef74677 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -2329,6 +2329,7 @@ int freeze_super(struct super_block *sb, enum freeze_holder who, const void *fre
>  	 */
>  	WARN_ON_ONCE(freeze_inc(sb, who) > 1);
>  	sb->s_writers.freeze_owner = freeze_owner;
> +	sb->s_writers.freeze_rwsems_locked = true;
>  	sb->s_writers.frozen = SB_FREEZE_COMPLETE;
>  	wake_up_var(&sb->s_writers.frozen);
>  	lockdep_sb_freeze_release(sb);
> @@ -2364,7 +2365,7 @@ static int thaw_super_locked(struct super_block *sb, enum freeze_holder who,
>  		goto out_unlock;
>  	}
>  
> -	if (sb_rdonly(sb)) {
> +	if (!sb->s_writers.freeze_rwsems_locked) {
>  		sb->s_writers.frozen = SB_UNFROZEN;
>  		sb->s_writers.freeze_owner = NULL;
>  		wake_up_var(&sb->s_writers.frozen);
> @@ -2387,6 +2388,7 @@ static int thaw_super_locked(struct super_block *sb, enum freeze_holder who,
>  	sb->s_writers.freeze_owner = NULL;
>  	wake_up_var(&sb->s_writers.frozen);
>  	sb_freeze_unlock(sb, SB_FREEZE_FS);
> +	sb->s_writers.freeze_rwsems_locked = false;
>  out_deactivate:
>  	deactivate_locked_super(sb);
>  	return 0;
> diff --git a/include/linux/fs/super_types.h b/include/linux/fs/super_types.h
> index ecd96aeb1cee7..3e15efab65329 100644
> --- a/include/linux/fs/super_types.h
> +++ b/include/linux/fs/super_types.h
> @@ -53,6 +53,7 @@ enum {
>  
>  struct sb_writers {
>  	unsigned short			frozen;		/* Is sb frozen? */
> +	bool				freeze_rwsems_locked; /* Freeze holds writer rwsems */
>  	int				freeze_kcount;	/* How many kernel freeze requests? */
>  	int				freeze_ucount;	/* How many userspace freeze requests? */
>  	const void			*freeze_owner;	/* Owner of the freeze */
> -- 
> 2.53.0
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR