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

[liubaolin]

在 2026/9/16 19:57, Namjae Jeon 写道:
> On Tue, Sep 15, 2026 at 11:38 AM Hongling Zeng <zenghongling@xxxxxxxxxx> 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.