Re: [PATCH] ntfs: fix lost volume flag updates in set/clear helpers

[Hongling Zeng]

在 2026年09月02日 09:06, Namjae Jeon 写道:
> On Tue, Sep 1, 2026 at 11:46 AM Hongling Zeng <zenghongling@xxxxxxxxxx> wrote:
> > ntfs_set_volume_flags() and ntfs_clear_volume_flags() both read
> > vol->vol_flags outside any lock to compute the new value before handing
> > it to ntfs_write_volume_flags(), which only takes ni->mrec_lock around
> > the actual write. The read-modify-write is therefore not atomic, and two
> > concurrent callers can lose an update: ntfs_sync_fs() may derive a
> > "clean" value from vol->vol_flags while a writer concurrently records an
> > error and sets VOLUME_IS_DIRTY; the locked write then silently
> > overwrites the freshly-set dirty bit. The on-disk volume looks clean
> > despite the recorded errors, so chkdsk will not run on the next mount
> > and corrupted metadata can persist.
> This patch description explicitly identifies ntfs_sync_fs() as the
> problematic caller, but there still remains a racy issue after
> applying this patch.
>
> static int ntfs_sync_fs(struct super_block *sb, int wait)
> {
> ...
>          if (!NVolErrors(vol) &&
>              ntfs_clear_volume_flags(vol, VOLUME_IS_DIRTY)) {
>                  ntfs_warning(sb, "Failed to clear dirty bit in volume
> information flags.  Run chkdsk.");
>                  err = -EIO;
>          }
>
> Sync thread                         Writer thread
> -----------                                -------------
> 1. NVolErrors() == false
>                                               2. Set VOLUME_IS_DIRTY
> under mrec_lock
>                                               3. NVolSetErrors()
> 4. ntfs_clear_volume_flags()
>
> The volume can be left marked clean on disk despite the recorded
> error... Can you update this patch to fix this issue as well?
|You're right. My current patch fixes the lost update in the set/clear
helpers, but ntfs_sync_fs() still has a TOCTOU race between NVolErrors()
and clearing VOLUME_IS_DIRTY.

I'll update the patch to make the error check and dirty-bit clear happen
under the same mrec_lock, so the sync path won't clear the bit after a
concurrent error update.

Thanks |