* Re: [PATCH v6] ntfs: mount hibernated volumes read-only regardless of errors=
2026-09-21 9:19 [PATCH v6] ntfs: mount hibernated volumes read-only regardless of errors= Hongling Zeng
@ 2026-09-22 0:18 ` liubaolin
2026-09-22 1:27 ` Hyunchul Lee
2026-09-22 1:42 ` liubaolin
2 siblings, 0 replies; 4+ messages in thread
From: liubaolin @ 2026-09-22 0:18 UTC (permalink / raw)
To: Hongling Zeng, linkinjeon, hyc.lee
Cc: ntfs, linux-kernel, zhongling0719, stable, Baolin Liu
在 2026/9/21 17:19, Hongling Zeng 写道:
> The hibernation check in load_system_files() only converts the
> superblock to read-only under errors=remount-ro. With the default
> errors=continue (and with errors=panic), a hibernated volume is
> mounted read-write and the mount-time $LogFile emptying writes to it,
> although a hibernated volume must not be written to at all.
>
> Drop the on_errors term so that a hibernated volume, or a volume whose
> hibernation state cannot be determined, always mounts read-only.
> NVolErrors() is still recorded, so ntfs_reconfigure() keeps refusing
> remounts to read-write, and the $LogFile emptying is skipped by its
> !sb_rdonly() check.
>
> The check itself must not apply the errors= policy either: it runs
> before SB_RDONLY is set, and ntfs_lookup_inode_by_name() and ntfs_iget()
> call ntfs_error() internally on corruption or I/O errors, which under
> errors=panic would panic the machine before the read-only fallback has
> made its decision. Temporarily substitute errors=remount-ro for
> errors=panic around the check: nested ntfs_error() calls then fall
> back to read-only, and any read-only transition they trigger is
> preserved even when the error is not propagated to the check's return
> value or recorded in NVolErrors(). This is safe during initial mount,
> before the super block is published.
>
> If the check reports hibernation or fails, set SB_RDONLY and record
> NVolErrors() unconditionally: hibernation safety takes precedence over
> the errors= policy. A volume with errors recorded during the check or
> earlier in the mount, e.g. when loading the LogFile, also stays
> read-only, matching what ntfs_reconfigure() enforces for remounts.
>
> Fixes: 6251f0b0de7d ("ntfs: update super block operations")
> Cc: stable@vger.kernel.org
> Suggested-by: Baolin Liu <liubaolin@kylinos.cn>
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
> Change in v6:
> - Temporarily substitute errors=remount-ro for errors=panic during
> the hibernation check. ntfs_error() does not always set
> NVolErrors(), so the v5 restore condition could re-enable write
> access after a real error.
> ---
> fs/ntfs/super.c | 25 ++++++++++++++++++++-----
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> index f4a73e45773d..da2c8f310a7f 100644
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
> @@ -1398,6 +1398,7 @@ static bool load_system_files(struct ntfs_volume *vol)
> struct ntfs_attr_search_ctx *ctx;
> struct restart_page_header *rp;
> int err;
> + u8 saved_on_errors;
>
> ntfs_debug("Entering.");
> /* Get mft mirror inode compare the contents of $MFT and $MFTMirr. */
> @@ -1572,8 +1573,19 @@ static bool load_system_files(struct ntfs_volume *vol)
> * NVolErrors() without setting the dirty volume flag and mount
> * read-only. This will prevent read-write remounting and it will also
> * prevent all writes.
> + *
> + * Nested lookup and inode-loading errors must not panic before the
> + * read-only fallback has run. Temporarily use errors=remount-ro
> + * instead of errors=panic, preserving any read-only transition even
> + * if an error is not propagated to the check's return value or
> + * recorded in NVolErrors(). This is safe during initial mount,
> + * before the super block is published.
> */
> + saved_on_errors = vol->on_errors;
> + if (saved_on_errors == ON_ERRORS_PANIC)
> + vol->on_errors = ON_ERRORS_REMOUNT_RO;
> err = check_windows_hibernation_status(vol);
> + vol->on_errors = saved_on_errors;
> if (unlikely(err)) {
> static const char *es1a = "Failed to determine if Windows is hibernated";
> static const char *es1b = "Windows is hibernated";
> @@ -1581,12 +1593,15 @@ static bool load_system_files(struct ntfs_volume *vol)
> const char *es1;
>
> es1 = err < 0 ? es1a : es1b;
> - /* If a read-write mount, convert it to a read-only mount. */
> - if (!sb_rdonly(sb) && vol->on_errors == ON_ERRORS_REMOUNT_RO) {
> - sb->s_flags |= SB_RDONLY;
> - ntfs_error(sb, "%s. Mounting read-only%s", es1, es2);
> - }
> + /* Hibernation safety takes precedence over the errors= policy. */
> + sb->s_flags |= SB_RDONLY;
> NVolSetErrors(vol);
> + ntfs_error(sb, "%s. Mounting read-only%s", es1, es2);
> + } else if (!sb_rdonly(sb) && NVolErrors(vol)) {
> + /* Match the read-write remount restriction for recorded errors. */
> + sb->s_flags |= SB_RDONLY;
> + ntfs_error(sb,
> + "Errors were recorded during mount. Mounting read-only. Run chkdsk.");
> }
>
> /* If (still) a read-write mount, empty the logfile. */
Looks good to me.
Reviewed-by: Baolin Liu <liubaolin@kylinos.cn>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v6] ntfs: mount hibernated volumes read-only regardless of errors=
2026-09-21 9:19 [PATCH v6] ntfs: mount hibernated volumes read-only regardless of errors= Hongling Zeng
2026-09-22 0:18 ` liubaolin
@ 2026-09-22 1:27 ` Hyunchul Lee
2026-09-22 1:42 ` liubaolin
2 siblings, 0 replies; 4+ messages in thread
From: Hyunchul Lee @ 2026-09-22 1:27 UTC (permalink / raw)
To: Hongling Zeng
Cc: linkinjeon, ntfs, linux-kernel, zhongling0719, stable, Baolin Liu
2026년 9월 21일 (월) 오후 6:19, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
>
> The hibernation check in load_system_files() only converts the
> superblock to read-only under errors=remount-ro. With the default
> errors=continue (and with errors=panic), a hibernated volume is
> mounted read-write and the mount-time $LogFile emptying writes to it,
> although a hibernated volume must not be written to at all.
>
> Drop the on_errors term so that a hibernated volume, or a volume whose
> hibernation state cannot be determined, always mounts read-only.
> NVolErrors() is still recorded, so ntfs_reconfigure() keeps refusing
> remounts to read-write, and the $LogFile emptying is skipped by its
> !sb_rdonly() check.
>
> The check itself must not apply the errors= policy either: it runs
> before SB_RDONLY is set, and ntfs_lookup_inode_by_name() and ntfs_iget()
> call ntfs_error() internally on corruption or I/O errors, which under
> errors=panic would panic the machine before the read-only fallback has
> made its decision. Temporarily substitute errors=remount-ro for
> errors=panic around the check: nested ntfs_error() calls then fall
> back to read-only, and any read-only transition they trigger is
> preserved even when the error is not propagated to the check's return
> value or recorded in NVolErrors(). This is safe during initial mount,
> before the super block is published.
>
> If the check reports hibernation or fails, set SB_RDONLY and record
> NVolErrors() unconditionally: hibernation safety takes precedence over
> the errors= policy. A volume with errors recorded during the check or
> earlier in the mount, e.g. when loading the LogFile, also stays
> read-only, matching what ntfs_reconfigure() enforces for remounts.
>
> Fixes: 6251f0b0de7d ("ntfs: update super block operations")
> Cc: stable@vger.kernel.org
> Suggested-by: Baolin Liu <liubaolin@kylinos.cn>
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
Looks good to me.
Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>
> ---
> Change in v6:
> - Temporarily substitute errors=remount-ro for errors=panic during
> the hibernation check. ntfs_error() does not always set
> NVolErrors(), so the v5 restore condition could re-enable write
> access after a real error.
> ---
> fs/ntfs/super.c | 25 ++++++++++++++++++++-----
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> index f4a73e45773d..da2c8f310a7f 100644
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
> @@ -1398,6 +1398,7 @@ static bool load_system_files(struct ntfs_volume *vol)
> struct ntfs_attr_search_ctx *ctx;
> struct restart_page_header *rp;
> int err;
> + u8 saved_on_errors;
>
> ntfs_debug("Entering.");
> /* Get mft mirror inode compare the contents of $MFT and $MFTMirr. */
> @@ -1572,8 +1573,19 @@ static bool load_system_files(struct ntfs_volume *vol)
> * NVolErrors() without setting the dirty volume flag and mount
> * read-only. This will prevent read-write remounting and it will also
> * prevent all writes.
> + *
> + * Nested lookup and inode-loading errors must not panic before the
> + * read-only fallback has run. Temporarily use errors=remount-ro
> + * instead of errors=panic, preserving any read-only transition even
> + * if an error is not propagated to the check's return value or
> + * recorded in NVolErrors(). This is safe during initial mount,
> + * before the super block is published.
> */
> + saved_on_errors = vol->on_errors;
> + if (saved_on_errors == ON_ERRORS_PANIC)
> + vol->on_errors = ON_ERRORS_REMOUNT_RO;
> err = check_windows_hibernation_status(vol);
> + vol->on_errors = saved_on_errors;
> if (unlikely(err)) {
> static const char *es1a = "Failed to determine if Windows is hibernated";
> static const char *es1b = "Windows is hibernated";
> @@ -1581,12 +1593,15 @@ static bool load_system_files(struct ntfs_volume *vol)
> const char *es1;
>
> es1 = err < 0 ? es1a : es1b;
> - /* If a read-write mount, convert it to a read-only mount. */
> - if (!sb_rdonly(sb) && vol->on_errors == ON_ERRORS_REMOUNT_RO) {
> - sb->s_flags |= SB_RDONLY;
> - ntfs_error(sb, "%s. Mounting read-only%s", es1, es2);
> - }
> + /* Hibernation safety takes precedence over the errors= policy. */
> + sb->s_flags |= SB_RDONLY;
> NVolSetErrors(vol);
> + ntfs_error(sb, "%s. Mounting read-only%s", es1, es2);
> + } else if (!sb_rdonly(sb) && NVolErrors(vol)) {
> + /* Match the read-write remount restriction for recorded errors. */
> + sb->s_flags |= SB_RDONLY;
> + ntfs_error(sb,
> + "Errors were recorded during mount. Mounting read-only. Run chkdsk.");
> }
>
> /* If (still) a read-write mount, empty the logfile. */
> --
> 2.25.1
>
--
Thanks,
Hyunchul
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v6] ntfs: mount hibernated volumes read-only regardless of errors=
2026-09-21 9:19 [PATCH v6] ntfs: mount hibernated volumes read-only regardless of errors= Hongling Zeng
2026-09-22 0:18 ` liubaolin
2026-09-22 1:27 ` Hyunchul Lee
@ 2026-09-22 1:42 ` liubaolin
2 siblings, 0 replies; 4+ messages in thread
From: liubaolin @ 2026-09-22 1:42 UTC (permalink / raw)
To: Hongling Zeng, linkinjeon, hyc.lee
Cc: ntfs, linux-kernel, zhongling0719, stable, Baolin Liu
在 2026/9/21 17:19, Hongling Zeng 写道:
> The hibernation check in load_system_files() only converts the
> superblock to read-only under errors=remount-ro. With the default
> errors=continue (and with errors=panic), a hibernated volume is
> mounted read-write and the mount-time $LogFile emptying writes to it,
> although a hibernated volume must not be written to at all.
>
> Drop the on_errors term so that a hibernated volume, or a volume whose
> hibernation state cannot be determined, always mounts read-only.
> NVolErrors() is still recorded, so ntfs_reconfigure() keeps refusing
> remounts to read-write, and the $LogFile emptying is skipped by its
> !sb_rdonly() check.
>
> The check itself must not apply the errors= policy either: it runs
> before SB_RDONLY is set, and ntfs_lookup_inode_by_name() and ntfs_iget()
> call ntfs_error() internally on corruption or I/O errors, which under
> errors=panic would panic the machine before the read-only fallback has
> made its decision. Temporarily substitute errors=remount-ro for
> errors=panic around the check: nested ntfs_error() calls then fall
> back to read-only, and any read-only transition they trigger is
> preserved even when the error is not propagated to the check's return
> value or recorded in NVolErrors(). This is safe during initial mount,
> before the super block is published.
>
> If the check reports hibernation or fails, set SB_RDONLY and record
> NVolErrors() unconditionally: hibernation safety takes precedence over
> the errors= policy. A volume with errors recorded during the check or
> earlier in the mount, e.g. when loading the LogFile, also stays
> read-only, matching what ntfs_reconfigure() enforces for remounts.
>
> Fixes: 6251f0b0de7d ("ntfs: update super block operations")
> Cc: stable@vger.kernel.org
> Suggested-by: Baolin Liu <liubaolin@kylinos.cn>
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
> Change in v6:
> - Temporarily substitute errors=remount-ro for errors=panic during
> the hibernation check. ntfs_error() does not always set
> NVolErrors(), so the v5 restore condition could re-enable write
> access after a real error.
> ---
> fs/ntfs/super.c | 25 ++++++++++++++++++++-----
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> index f4a73e45773d..da2c8f310a7f 100644
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
> @@ -1398,6 +1398,7 @@ static bool load_system_files(struct ntfs_volume *vol)
> struct ntfs_attr_search_ctx *ctx;
> struct restart_page_header *rp;
> int err;
> + u8 saved_on_errors;
>
> ntfs_debug("Entering.");
> /* Get mft mirror inode compare the contents of $MFT and $MFTMirr. */
> @@ -1572,8 +1573,19 @@ static bool load_system_files(struct ntfs_volume *vol)
> * NVolErrors() without setting the dirty volume flag and mount
> * read-only. This will prevent read-write remounting and it will also
> * prevent all writes.
> + *
> + * Nested lookup and inode-loading errors must not panic before the
> + * read-only fallback has run. Temporarily use errors=remount-ro
> + * instead of errors=panic, preserving any read-only transition even
> + * if an error is not propagated to the check's return value or
> + * recorded in NVolErrors(). This is safe during initial mount,
> + * before the super block is published.
> */
> + saved_on_errors = vol->on_errors;
> + if (saved_on_errors == ON_ERRORS_PANIC)
> + vol->on_errors = ON_ERRORS_REMOUNT_RO;
> err = check_windows_hibernation_status(vol);
> + vol->on_errors = saved_on_errors;
> if (unlikely(err)) {
> static const char *es1a = "Failed to determine if Windows is hibernated";
> static const char *es1b = "Windows is hibernated";
> @@ -1581,12 +1593,15 @@ static bool load_system_files(struct ntfs_volume *vol)
> const char *es1;
>
> es1 = err < 0 ? es1a : es1b;
> - /* If a read-write mount, convert it to a read-only mount. */
> - if (!sb_rdonly(sb) && vol->on_errors == ON_ERRORS_REMOUNT_RO) {
> - sb->s_flags |= SB_RDONLY;
> - ntfs_error(sb, "%s. Mounting read-only%s", es1, es2);
> - }
> + /* Hibernation safety takes precedence over the errors= policy. */
> + sb->s_flags |= SB_RDONLY;
> NVolSetErrors(vol);
> + ntfs_error(sb, "%s. Mounting read-only%s", es1, es2);
> + } else if (!sb_rdonly(sb) && NVolErrors(vol)) {
> + /* Match the read-write remount restriction for recorded errors. */
> + sb->s_flags |= SB_RDONLY;
> + ntfs_error(sb,
> + "Errors were recorded during mount. Mounting read-only. Run chkdsk.");
> }
>
> /* If (still) a read-write mount, empty the logfile. */
Hi Hongling,
The change itself looks good to me, and I'm happy to provide a
Reviewed-by. However, v6 does not apply cleanly to the latest ntfs-next.
Commit 86e6932550ba ("ntfs: sync the volume dirty bit with the
recorded error state") added the following after NVolSetErrors(vol) in
the hibernation handling in fs/ntfs/super.c:
/*
* Remember it for the lifetime of the mount: see
* ntfs_sync_volume_dirty_state().
*/
NVolSetHibernated(vol);
The third hunk of v6 expects the closing brace immediately after
NVolSetErrors(vol), so its context no longer matches.
Could you please rebase the patch onto the latest ntfs-next, keeping
NVolSetHibernated(vol) and its comment inside the if (unlikely(err))
branch? This flag prevents subsequent dirty-bit synchronization from
writing to a hibernated volume, so it needs to be preserved.
This is a context conflict; the hibernation fix itself is still needed.
With the existing flag handling preserved, please feel free to add:
Reviewed-by: Baolin Liu <liubaolin@kylinos.cn>
Thanks,
Baolin.
^ permalink raw reply [flat|nested] 4+ messages in thread