Re: [PATCH v3 11/33] exfat: don't set SB_RDONLY after filesystem errors

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] exfat: exfat_page_mkwrite() checks emergency state and
  dirties before sb_start_pagefault()
--

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

exfat: don't set SB_RDONLY after filesystem errors

This commit replaces setting SB_RDONLY on runtime filesystem errors with an
EXFAT_FLAGS_EMERGENCY_RO flag and checks exfat_emergency_state() across write
and modification paths so frozen filesystems can thaw cleanly.

> diff --git a/fs/exfat/file.c b/fs/exfat/file.c
> index 1386149916bbe..dddedf6e75da6 100644
> --- a/fs/exfat/file.c
> +++ b/fs/exfat/file.c
[ ... ]
> @@ -974,6 +987,11 @@ static vm_fault_t exfat_page_mkwrite(struct vm_fault *vmf)
>  	struct exfat_inode_info *ei = EXFAT_I(inode);
>  	vm_fault_t ret;
>  	loff_t new_valid_size, mmap_valid_size, fault_page_start;
> +	int err;
> +
> +	err = exfat_emergency_state(inode->i_sb);
> +	if (unlikely(err))
> +		return vmf_fs_error(err);
>  
>  	if (!inode_trylock(inode))
>  		return VM_FAULT_RETRY;

[Severity: Medium]
Can a write fault that blocks in sb_start_pagefault() bypass
exfat_emergency_state() if a filesystem error or shutdown happens while the
filesystem is frozen?

CPU0                                    CPU1
exfat_page_mkwrite()
  exfat_emergency_state(sb) -> 0
  inode_trylock(inode)
                                        bdev_freeze()
                                        ...
                                        __exfat_fs_error()
                                          set_bit(EXFAT_FLAGS_EMERGENCY_RO, ...)
  sb_start_pagefault(sb)
    /* waits for thaw */
                                        bdev_thaw()
  file_update_time(...)
  iomap_page_mkwrite(...)               <-- installs writable PTE after error

Because sb_start_pagefault() is called near the bottom of
exfat_page_mkwrite(), after exfat_prepare_valid_range() and
mark_inode_dirty(), a fault arriving on an already-frozen filesystem can
also dirty gap folios and the inode while frozen and then sleep in
sb_start_pagefault() while holding inode->i_rwsem:

fs/exfat/file.c:exfat_page_mkwrite() {
    ...
		exfat_advance_zeroed_size(ei,
				round_up(new_valid_size, i_blocksize(inode)));
		exfat_set_valid_size(ei, new_valid_size);
		mark_inode_dirty(inode);
	}

	sb_start_pagefault(inode->i_sb);
	file_update_time(vmf->vma->vm_file);

	filemap_invalidate_lock_shared(inode->i_mapping);
	ret = iomap_page_mkwrite(vmf, &exfat_iomap_ops, NULL);
    ...
}

Calling sb_start_pagefault() before inode_trylock() and checking
exfat_emergency_state() after acquiring freeze protection would avoid both
dirtying state while frozen and missing an error that occurred during the
freeze.

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