Re: [PATCH v3 19/33] ntfs: 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] ntfs: unlocked sb->s_flags RMW in ntfs_handle_error() via
  precalc_work
--

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

ntfs: don't set SB_RDONLY after filesystem errors

This commit replaces setting SB_RDONLY on runtime filesystem errors under
errors=remount-ro with an NV_EmergencyRO volume flag and an asynchronous
work item to persist the volume dirty bit.

> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> index 71f0aea46bf8f..91290688d2898 100644
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
[ ... ]
> @@ -434,7 +436,21 @@ void ntfs_handle_error(struct super_block *sb)
>  		return;
>  
>  	if (vol->on_errors == ON_ERRORS_REMOUNT_RO) {
> -		sb->s_flags |= SB_RDONLY;
> +		if (!(sb->s_flags & SB_BORN)) {
> +			/* Before SB_BORN the mounter holds s_umount. */
> +			sb->s_flags |= SB_RDONLY;
> +		} else {

[Severity: Medium]
Can precalc_free_clusters() reach this branch on a kworker thread before
SB_BORN is set, racing on sb->s_flags without holding s_umount?

ntfs_fill_super() queues vol->precalc_work before returning to
get_tree_bdev_flags() and vfs_get_tree():

CPU0 (mounter)                    CPU1 (kworker)
vfs_get_tree()
  get_tree_bdev_flags()
    ntfs_fill_super()
      queue_work(..., &vol->precalc_work)
                                  precalc_free_clusters()
                                    get_nr_free_clusters()
                                      ...
                                        ntfs_error()
                                          ntfs_handle_error()
                                            /* !(sb->s_flags & SB_BORN) */
  super_wake(sb, SB_BORN)                   sb->s_flags |= SB_RDONLY;
    smp_store_release(&sb->s_flags,           <-- unlocked RMW race
                      sb->s_flags | SB_BORN);

During mount, load_system_files() does not populate the runlist for
$Bitmap, so get_nr_free_clusters() decompresses $Bitmap's mapping pairs on
the workqueue and calls ntfs_error() if reading or decompressing fails.

Because neither side updates sb->s_flags atomically and the kworker does
not hold s_umount, the RMW in ntfs_handle_error() can overwrite SB_BORN
(leaving later super_lock() callers hung waiting for SB_BORN) or lose
SB_RDONLY.  Taking the !SB_BORN branch from the workqueue also skips
NVolSetErrors(vol) and NV_EmergencyRO, allowing a subsequent remount,rw in
ntfs_reconfigure() to succeed despite the error.

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