From 1a3883b3685cc4c8bc41927cc9df539239d7c961 Mon Sep 17 00:00:00 2001 From: Hongling Zeng Date: Tue, 15 Sep 2026 16:32:51 +0800 Subject: [PATCH] ntfs: check the dirty-state commit on remount and unmount The remount-to-read-only path commits the updated volume flags with ntfs_commit_inode(), a void wrapper around __ntfs_write_inode(), and ignores the blkdev_issue_flush() return value, so a failed commit or flush is reported as success. Once the remount has succeeded no persistence point is ever reached again: ntfs_put_super() skips read-only superblocks and the VFS never syncs one, so fail the remount unless the commit and the flush succeed. The superblock then stays read-write and ntfs_put_super() retries the persistence at unmount. The errors=remount-ro downgrade does not go through ntfs_reconfigure() and is unchanged. A zero-return commit is not trusted blindly: write_mft_record() redirties the record on allocation failure and reports success, so the $Volume inode is required to be clean afterwards. ntfs_put_super() discards the same commit error. Call __ntfs_write_inode() there with the same dirty re-check and warn on failure, as put_super() cannot return an error. The commit is skipped when the dirty-state sync itself failed, as that could write back an inconsistent flag state; a record left dirty by an earlier update is still committed at evict time. NVolErrors() is deliberately not used to detect the failure: it is sticky for the lifetime of the mount, so it cannot distinguish a fresh commit failure from errors recorded before the remount. Hibernated volumes: ntfs_sync_volume_dirty_state() is a no-op for them and never dirties the $Volume inode on such a mount, since the on-disk flags are already dirty and ntfs_set_volume_flags() has nothing to change. The commit only runs if something dirtied the inode independently, as before this patch; mounting hibernated volumes read-only removes even that. Reported-by: Baolin Liu Cc: stable@vger.kernel.org Signed-off-by: Hongling Zeng Reviewed-by: Hyunchul Lee Reviewed-by: Baolin Liu Signed-off-by: Namjae Jeon --- fs/ntfs/super.c | 76 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 22 deletions(-) diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c index a08523457fe0..0ec4d2ddf621 100644 --- a/fs/ntfs/super.c +++ b/fs/ntfs/super.c @@ -269,6 +269,7 @@ static int ntfs_reconfigure(struct fs_context *fc) { struct super_block *sb = fc->root->d_sb; struct ntfs_volume *vol = NTFS_SB(sb); + int err; ntfs_debug("Entering with remount"); @@ -331,14 +332,39 @@ static int ntfs_reconfigure(struct fs_context *fc) * and ntfs_put_super() skips them, so the only remaining * write would be the evict-time commit at unmount, which * a crash never reaches. An error recorded only after - * the remount is still never persisted. + * the remount is still never persisted; a failed commit + * or flush fails the remount, leaving the superblock + * read-write so ntfs_put_super() retries at unmount. */ - if (ntfs_sync_volume_dirty_state(vol)) { + err = ntfs_sync_volume_dirty_state(vol); + if (err) { ntfs_warning(sb, "Failed to update dirty bit in volume information flags. Run chkdsk."); - } else if (NInoDirty(NTFS_I(vol->vol_ino))) { - ntfs_commit_inode(vol->vol_ino); - blkdev_issue_flush(sb->s_bdev); + return err; + } + if (NInoDirty(NTFS_I(vol->vol_ino))) { + /* ntfs_commit_inode() would discard the error. */ + err = __ntfs_write_inode(vol->vol_ino, 1); + if (err) { + ntfs_warning(sb, + "Failed to commit volume information flags. Run chkdsk."); + return err; + } + /* + * write_mft_record() redirties the record on + * -ENOMEM and still reports success. + */ + if (NInoDirty(NTFS_I(vol->vol_ino))) { + ntfs_warning(sb, + "Volume information flags remain dirty after commit. Run chkdsk."); + return -EIO; + } + err = blkdev_issue_flush(sb->s_bdev); + if (err) { + ntfs_warning(sb, + "Failed to flush volume information flags. Run chkdsk."); + return err; + } } } @@ -1944,26 +1970,32 @@ static void ntfs_put_super(struct super_block *sb) if (ntfs_sync_volume_dirty_state(vol)) { ntfs_warning(sb, "Failed to sync dirty bit in volume information flags. Run chkdsk."); - } else if (NVolErrors(vol)) { + } else { /* - * The dirty bit is on disk now; only warn when the - * sync actually succeeded, or this message would - * contradict the one above. + * __ntfs_write_inode(), not the void + * ntfs_commit_inode() wrapper: the error can only + * be warned about here. The mirror inode is only + * released below: writing the $Volume record (mft + * record number 3, below vol->mftmirr_size) mirrors + * it through ntfs_sync_mft_mirror(), which fails + * with -EIO once vol->mftmirr_ino is gone. */ - ntfs_warning(sb, - "Volume has errors. Leaving volume marked dirty. Run chkdsk."); + if (__ntfs_write_inode(vol->vol_ino, 1)) { + ntfs_warning(sb, + "Failed to commit volume information flags. Run chkdsk."); + } else if (NInoDirty(NTFS_I(vol->vol_ino))) { + ntfs_warning(sb, + "Volume information flags remain dirty after commit. Run chkdsk."); + } else if (NVolErrors(vol)) { + /* + * Only warn once the commit has succeeded, + * or this could contradict a failure + * reported above. + */ + ntfs_warning(sb, + "Volume has errors. Leaving volume marked dirty. Run chkdsk."); + } } - /* - * Commits the updated volume flags if they were written. - * The mft mirror must still be around for this: the - * $Volume record (mft record number 3, below - * vol->mftmirr_size) is mirrored by write_mft_record() - * through ntfs_sync_mft_mirror(), which fails with -EIO - * and leaves the mirror stale once vol->mftmirr_ino is - * gone, so the mirror inode is only released after this - * commit. - */ - ntfs_commit_inode(vol->vol_ino); } /*