[v11] ntfs: fix volume flag races and persist the recorded error state

[PATCH v11 0/6] ntfs: fix volume flag races and persist the recorded error state

Posted by Hongling Zeng 1 week, 4 days ago

The fs/ntfs runtime metadata-corruption paths only record the in-memory
NVolErrors() flag, and the caller-side dirty-bit marking races with
ntfs_sync_fs(): a volume can end up with a clean on-disk dirty flag
despite modification or recorded corruption, so chkdsk never runs on
the next mount.  Based on ntfs/ntfs-next (9a05b5715cfa).

 1/4 makes the volume flag read-modify-write atomic under the
    $Volume mrec_lock;
 2/4 marks the volume dirty unconditionally on metadata changes,
    dropping the racy caller-side checks in file.c and namei.c;
 3/4 derives the on-disk dirty bit from the recorded error state at
    the persistence points (sync_fs, remount-ro, put_super) and never
    writes a hibernated volume;
 4/4 persists the dirty state after the final put_super() commits so
    late errors cannot unmount clean.

Changes in v11:

- Add 5/6: Stop clearing VOLUME_IS_DIRTY during sync. The bit is now
  cleared only on remount to read-only or clean unmount, preventing a
  crash during metadata updates from bypassing chkdsk. Verified in QEMU.

- Add 6/6: Check commit and flush errors during remount and unmount.
  Re-check NInoDirty() after __ntfs_write_inode(). A failed remount is
  rejected so unmount can retry; unmount failures are reported as warnings.


Hongling Zeng (4):
  ntfs: fix volume flag update races
  ntfs: set the volume dirty bit unconditionally on metadata changes
  ntfs: sync the volume dirty bit with the recorded error state
  ntfs: persist the dirty state after the final put_super() commits

 fs/ntfs/file.c   |  20 ++---
 fs/ntfs/namei.c  |  24 ++----
 fs/ntfs/ntfs.h   |   1 -
 fs/ntfs/super.c  | 191 ++++++++++++++++++++++++++++++++++++-----------
 fs/ntfs/volume.h |   4 +
 5 files changed, 171 insertions(+), 69 deletions(-)

-- 
2.25.1

Re: [PATCH v11 0/6] ntfs: fix volume flag races and persist the recorded error state

Posted by Hyunchul Lee 1 week, 3 days ago

Hi Hongling,

Thank you, and this patch set looks good to me.

As a follow up, it would be useful to discuss making volume dirty
when an RW mount is established and clearing it only after a clean
unmount.

The current implementation still has a window where metadata
changes may be written while the on-disk volume flag is still
clean, and also does not clear the dirty volume for sync(2).

2026년 9월 14일 (월) 오후 2:49, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
>
> The fs/ntfs runtime metadata-corruption paths only record the in-memory
> NVolErrors() flag, and the caller-side dirty-bit marking races with
> ntfs_sync_fs(): a volume can end up with a clean on-disk dirty flag
> despite modification or recorded corruption, so chkdsk never runs on
> the next mount.  Based on ntfs/ntfs-next (9a05b5715cfa).
>
>  1/4 makes the volume flag read-modify-write atomic under the
>     $Volume mrec_lock;
>  2/4 marks the volume dirty unconditionally on metadata changes,
>     dropping the racy caller-side checks in file.c and namei.c;
>  3/4 derives the on-disk dirty bit from the recorded error state at
>     the persistence points (sync_fs, remount-ro, put_super) and never
>     writes a hibernated volume;
>  4/4 persists the dirty state after the final put_super() commits so
>     late errors cannot unmount clean.
>
> Changes in v11:
>
> - Add 5/6: Stop clearing VOLUME_IS_DIRTY during sync. The bit is now
>   cleared only on remount to read-only or clean unmount, preventing a
>   crash during metadata updates from bypassing chkdsk. Verified in QEMU.
>
> - Add 6/6: Check commit and flush errors during remount and unmount.
>   Re-check NInoDirty() after __ntfs_write_inode(). A failed remount is
>   rejected so unmount can retry; unmount failures are reported as warnings.
>
>
> Hongling Zeng (4):
>   ntfs: fix volume flag update races
>   ntfs: set the volume dirty bit unconditionally on metadata changes
>   ntfs: sync the volume dirty bit with the recorded error state
>   ntfs: persist the dirty state after the final put_super() commits
>
>  fs/ntfs/file.c   |  20 ++---
>  fs/ntfs/namei.c  |  24 ++----
>  fs/ntfs/ntfs.h   |   1 -
>  fs/ntfs/super.c  | 191 ++++++++++++++++++++++++++++++++++++-----------
>  fs/ntfs/volume.h |   4 +
>  5 files changed, 171 insertions(+), 69 deletions(-)
>
> --
> 2.25.1
>


-- 
Thanks,
Hyunchul

Re:Re: [PATCH v11 0/6] ntfs: fix volume flag races and persist the recorded error state

Posted by dd 1 week, 3 days ago


At 2026-09-15 13:48:05, "Hyunchul Lee" <hyc.lee@gmail.com> wrote:
>Hi Hongling,
>
>Thank you, and this patch set looks good to me.
>
>As a follow up, it would be useful to discuss making volume dirty
>when an RW mount is established and clearing it only after a clean
>unmount.
>
>The current implementation still has a window where metadata
>changes may be written while the on-disk volume flag is still
>clean, and also does not clear the dirty volume for sync(2).
>
Hi Hyunchul,

Thank you for the review and for confirming that the patch set looks
good.

I agree that marking the volume dirty when establishing an RW mount,
and clearing it only after a clean unmount, would close the remaining
window before the first metadata update. I also agree that sync(2)
should not clear the volume dirty flag, since the filesystem remains
writable after sync completes.

I will follow up with a separate patch/discussion for this behavior.

Thanks,
Hongling
>2026년 9월 14일 (월) 오후 2:49, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
>>
>> The fs/ntfs runtime metadata-corruption paths only record the in-memory
>> NVolErrors() flag, and the caller-side dirty-bit marking races with
>> ntfs_sync_fs(): a volume can end up with a clean on-disk dirty flag
>> despite modification or recorded corruption, so chkdsk never runs on
>> the next mount.  Based on ntfs/ntfs-next (9a05b5715cfa).
>>
>>  1/4 makes the volume flag read-modify-write atomic under the
>>     $Volume mrec_lock;
>>  2/4 marks the volume dirty unconditionally on metadata changes,
>>     dropping the racy caller-side checks in file.c and namei.c;
>>  3/4 derives the on-disk dirty bit from the recorded error state at
>>     the persistence points (sync_fs, remount-ro, put_super) and never
>>     writes a hibernated volume;
>>  4/4 persists the dirty state after the final put_super() commits so
>>     late errors cannot unmount clean.
>>
>> Changes in v11:
>>
>> - Add 5/6: Stop clearing VOLUME_IS_DIRTY during sync. The bit is now
>>   cleared only on remount to read-only or clean unmount, preventing a
>>   crash during metadata updates from bypassing chkdsk. Verified in QEMU.
>>
>> - Add 6/6: Check commit and flush errors during remount and unmount.
>>   Re-check NInoDirty() after __ntfs_write_inode(). A failed remount is
>>   rejected so unmount can retry; unmount failures are reported as warnings.
>>
>>
>> Hongling Zeng (4):
>>   ntfs: fix volume flag update races
>>   ntfs: set the volume dirty bit unconditionally on metadata changes
>>   ntfs: sync the volume dirty bit with the recorded error state
>>   ntfs: persist the dirty state after the final put_super() commits
>>
>>  fs/ntfs/file.c   |  20 ++---
>>  fs/ntfs/namei.c  |  24 ++----
>>  fs/ntfs/ntfs.h   |   1 -
>>  fs/ntfs/super.c  | 191 ++++++++++++++++++++++++++++++++++++-----------
>>  fs/ntfs/volume.h |   4 +
>>  5 files changed, 171 insertions(+), 69 deletions(-)
>>
>> --
>> 2.25.1
>>
>
>
>-- 
>Thanks,
>Hyunchul