[PATCH v3] ntfs: mount hibernated volumes read-only regardless of errors=

Hongling Zeng posted 1 patch 1 week, 3 days ago
There is a newer version of this series
fs/ntfs/super.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
[PATCH v3] ntfs: mount hibernated volumes read-only regardless of errors=
Posted by Hongling Zeng 1 week, 3 days ago
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.

Also change the ntfs_error() calls inside
check_windows_hibernation_status() to ntfs_warning(): they run before
SB_RDONLY is set, so errors=panic could panic there, while the warnings
preserve diagnostics for already read-only mounts.  The read-only
fallback message is logged unconditionally: with SB_RDONLY set, or on
an already read-only mount, ntfs_error() cannot panic, and the reason
for NVolErrors() stays visible.

Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

---
Changes in v3:
 -Use ntfs_warning() for the hibernation diagnostics so that
  errors=panic cannot fire on this path.
 -Always log the read-only fallback message, including on already
  read-only mounts.
---
 fs/ntfs/super.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index f4a73e45773d..d1edf1a891e8 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -1169,7 +1169,7 @@ static int check_windows_hibernation_status(struct ntfs_volume *vol)
 			return 0;
 		}
 		/* A real error occurred. */
-		ntfs_error(vol->sb, "Failed to find inode number for hiberfil.sys.");
+		ntfs_warning(vol->sb, "Failed to find inode number for hiberfil.sys.");
 		return ret;
 	}
 	/* Get the inode. */
@@ -1177,7 +1177,7 @@ static int check_windows_hibernation_status(struct ntfs_volume *vol)
 	if (IS_ERR(vi)) {
 		if (!IS_ERR(vi))
 			iput(vi);
-		ntfs_error(vol->sb, "Failed to load hiberfil.sys.");
+		ntfs_warning(vol->sb, "Failed to load hiberfil.sys.");
 		return IS_ERR(vi) ? PTR_ERR(vi) : -EIO;
 	}
 	if (unlikely(i_size_read(vi) < NTFS_HIBERFIL_HEADER_SIZE)) {
@@ -1188,7 +1188,7 @@ static int check_windows_hibernation_status(struct ntfs_volume *vol)
 
 	folio = read_mapping_folio(vi->i_mapping, 0, NULL);
 	if (IS_ERR(folio)) {
-		ntfs_error(vol->sb, "Failed to read from hiberfil.sys.");
+		ntfs_warning(vol->sb, "Failed to read from hiberfil.sys.");
 		ret = PTR_ERR(folio);
 		goto iput_out;
 	}
@@ -1581,11 +1581,16 @@ 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) {
+		/*
+		 * A Windows hibernation image is not a filesystem error, so
+		 * this is a safety interlock rather than something the
+		 * errors= policy may downgrade: always convert a read-write
+		 * mount to read-only.
+		 */
+		if (!sb_rdonly(sb))
 			sb->s_flags |= SB_RDONLY;
-			ntfs_error(sb, "%s.  Mounting read-only%s", es1, es2);
-		}
+
+		ntfs_error(sb, "%s.  Mounting read-only%s", es1, es2);
 		NVolSetErrors(vol);
 	}
 
-- 
2.25.1
Re: [PATCH v3] ntfs: mount hibernated volumes read-only regardless of errors=
Posted by Namjae Jeon 1 week, 1 day ago
On Tue, Sep 15, 2026 at 11:38 AM Hongling Zeng <zenghongling@kylinos.cn> wrote:
>
> 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.
>
> Also change the ntfs_error() calls inside
> check_windows_hibernation_status() to ntfs_warning(): they run before
> SB_RDONLY is set, so errors=panic could panic there, while the warnings
> preserve diagnostics for already read-only mounts.  The read-only
> fallback message is logged unconditionally: with SB_RDONLY set, or on
> an already read-only mount, ntfs_error() cannot panic, and the reason
> for NVolErrors() stays visible.
errors=panic can still panic before the read-only fallback.
ntfs_lookup_inode_by_name() and ntfs_iget() in
check_windows_hibernation_status() can call ntfs_error() internally.
Re: [PATCH v3] ntfs: mount hibernated volumes read-only regardless of errors=
Posted by liubaolin 1 week, 1 day ago

在 2026/9/16 19:57, Namjae Jeon 写道:
> On Tue, Sep 15, 2026 at 11:38 AM Hongling Zeng <zenghongling@kylinos.cn> wrote:
>>
>> 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.
>>
>> Also change the ntfs_error() calls inside
>> check_windows_hibernation_status() to ntfs_warning(): they run before
>> SB_RDONLY is set, so errors=panic could panic there, while the warnings
>> preserve diagnostics for already read-only mounts.  The read-only
>> fallback message is logged unconditionally: with SB_RDONLY set, or on
>> an already read-only mount, ntfs_error() cannot panic, and the reason
>> for NVolErrors() stays visible.
> errors=panic can still panic before the read-only fallback.
> ntfs_lookup_inode_by_name() and ntfs_iget() in
> check_windows_hibernation_status() can call ntfs_error() internally.

Hi Namjae and Hongling,

   Namjae,thank you for catching this, and sorry I missed those internal 
ntfs_error() calls in my earlier review.

   Changing the ntfs_error() calls in the shared lookup and 
inode-loading helpers to ntfs_warning() would affect other callers as 
well, so I would prefer to avoid that.

   Would it make sense to temporarily set SB_RDONLY before calling 
check_windows_hibernation_status()?
Since ntfs_handle_error() returns immediately for a read-only 
superblock, this would prevent the nested calls from triggering 
errors=panic, while preserving the existing error diagnostics. With this 
approach, the direct ntfs_error() calls in the hibernation check could 
also remain unchanged.load_system_files() is only called during the 
initial mount, before the filesystem is exposed to userspace.

The basic idea would be:

   bool temporary_ro = false;

   if (!sb_rdonly(sb)) {
   	sb->s_flags |= SB_RDONLY;
   	temporary_ro = true;
   }

   err = check_windows_hibernation_status(vol);

   if (temporary_ro && !err && !NVolErrors(vol))
   	sb->s_flags &= ~SB_RDONLY;

   If Windows is hibernated or the check fails, we would leave the 
volume read-only, call NVolSetErrors(vol), and log the fallback message 
unconditionally. A volume that was already read-only before the check 
would remain read-only.

   The reason for adding !NVolErrors(vol) is that 
ntfs_read_locked_inode() can continue after an error looking up 
AT_EA_INFORMATION. The underlying code may have already reported a 
metadata error and set NVolErrors(), while the hibernation check can 
still return zero. Checking !err alone could therefore restore 
read-write access despite the detected error. This additional condition 
is conservative: it would also keep volumes with previously recorded 
errors read-only, and that case would need a corresponding diagnostic.

   Namjae and Hongling,I would appreciate your thoughts on this 
approach, particularly on the condition for restoring read-write access.
   If you both agree that this approach is reasonable, Hongling, could 
you please update your patch based on this approach and send a new version?

Thanks,
Baolin.

Re: [PATCH v3] ntfs: mount hibernated volumes read-only regardless of errors=
Posted by liubaolin 1 week, 3 days ago

在 2026/9/15 10:38, 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.
> 
> Also change the ntfs_error() calls inside
> check_windows_hibernation_status() to ntfs_warning(): they run before
> SB_RDONLY is set, so errors=panic could panic there, while the warnings
> preserve diagnostics for already read-only mounts.  The read-only
> fallback message is logged unconditionally: with SB_RDONLY set, or on
> an already read-only mount, ntfs_error() cannot panic, and the reason
> for NVolErrors() stays visible.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

Hi Hongling,
The patch looks good to me. One suggestion: please add the following 
Fixes tag:
Fixes: 6251f0b0de7d ("ntfs: update super block operations")

Reviewed-by: Baolin Liu <liubaolin@kylinos.cn>

> 
> ---
> Changes in v3:
>   -Use ntfs_warning() for the hibernation diagnostics so that
>    errors=panic cannot fire on this path.
>   -Always log the read-only fallback message, including on already
>    read-only mounts.
> ---
>   fs/ntfs/super.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> index f4a73e45773d..d1edf1a891e8 100644
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
> @@ -1169,7 +1169,7 @@ static int check_windows_hibernation_status(struct ntfs_volume *vol)
>   			return 0;
>   		}
>   		/* A real error occurred. */
> -		ntfs_error(vol->sb, "Failed to find inode number for hiberfil.sys.");
> +		ntfs_warning(vol->sb, "Failed to find inode number for hiberfil.sys.");
>   		return ret;
>   	}
>   	/* Get the inode. */
> @@ -1177,7 +1177,7 @@ static int check_windows_hibernation_status(struct ntfs_volume *vol)
>   	if (IS_ERR(vi)) {
>   		if (!IS_ERR(vi))
>   			iput(vi);
> -		ntfs_error(vol->sb, "Failed to load hiberfil.sys.");
> +		ntfs_warning(vol->sb, "Failed to load hiberfil.sys.");
>   		return IS_ERR(vi) ? PTR_ERR(vi) : -EIO;
>   	}
>   	if (unlikely(i_size_read(vi) < NTFS_HIBERFIL_HEADER_SIZE)) {
> @@ -1188,7 +1188,7 @@ static int check_windows_hibernation_status(struct ntfs_volume *vol)
>   
>   	folio = read_mapping_folio(vi->i_mapping, 0, NULL);
>   	if (IS_ERR(folio)) {
> -		ntfs_error(vol->sb, "Failed to read from hiberfil.sys.");
> +		ntfs_warning(vol->sb, "Failed to read from hiberfil.sys.");
>   		ret = PTR_ERR(folio);
>   		goto iput_out;
>   	}
> @@ -1581,11 +1581,16 @@ 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) {
> +		/*
> +		 * A Windows hibernation image is not a filesystem error, so
> +		 * this is a safety interlock rather than something the
> +		 * errors= policy may downgrade: always convert a read-write
> +		 * mount to read-only.
> +		 */
> +		if (!sb_rdonly(sb))
>   			sb->s_flags |= SB_RDONLY;
> -			ntfs_error(sb, "%s.  Mounting read-only%s", es1, es2);
> -		}
> +
> +		ntfs_error(sb, "%s.  Mounting read-only%s", es1, es2);
>   		NVolSetErrors(vol);
>   	}
>