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

[liubaolin]

在 2026/9/21 17:30, dd 写道:
> Hi baolin,
>   Thanks for catching this. I agree that temporarily setting
>      SB_RDONLY
>      makes it impossible to distinguish an error suppressed by
>      ntfs_handle_error()
>      from a successful check. In particular, errors from optional WSL EA loading may neither propagate through the hibernation check nor set
>      NVolErrors()
>      , allowing the flag to be cleared incorrectly.
>
> Hi all:
>     One question: with errors=panic, a corrupt $LogFile can still panic
> during load_system_files(), before the hibernation fallback is reached.
> For example, ntfs_check_logfile() panics on "LogFile is too small".
>
>     Should mount-time metadata errors generally avoid errors=panic before
> the superblock is published? If so, that seems like a separate follow-up
> to extend the temporary policy substitution over load_system_files().
>
> Thanks!

Hi Hongling,

  Yes, the earlier $LogFile check can still panic. I would keep that 
behavior and limit the temporary substitution to the hibernation check.

  The intention of this exception is to preserve read-only access where 
possible when we cannot establish whether Windows is hibernated.Refusing 
write access provides a conservative outcome in that case, without 
bringing down the whole system. Temporarily substituting remount-ro lets 
us reach that outcome even when nested lookup or inode-loading helpers 
call ntfs_error().

  This deliberately also covers genuine metadata errors encountered 
during the probe. It is a limited policy exception for the hibernation 
check,rather than a requirement to suppress panic throughout mount.

  A corrupt $LogFile detected earlier is an independent filesystem 
error.Honoring the user's explicit errors=panic choice there is 
reasonable,and that path will not proceed to the subsequent $LogFile 
emptying.I do not think the hibernation exception requires changing that 
behavior.

  Also, extending the substitution over load_system_files() would not 
cover the whole mount process: boot-sector processing and the initial 
$MFT loading happen before it. Extending it further to cover all mount 
stages would effectively make errors=panic apply only after a successful 
mount, while using remount-ro during mount. I think that would override 
the user's selected policy too broadly.

  So my preference is to keep the temporary substitution around 
check_windows_hibernation_status() and preserve the existing policy 
elsewhere.

Thanks,
Baolin.


> At 2026-09-21 15:57:29, "Hyunchul Lee" <hyc.lee@xxxxxxxxx> wrote:
> > Hi Baolin, Hongling
> >
> > >     If you agree with this approach, could you please incorporate it and
> > > send another revision? I'd appreciate feedback from you, Namjae, and
> > > Hyunchul.
> >
> > I agree with this approach. It looks sound to me.
> >
> > > feel free to add:
> > > Suggested-by: Baolin Liu <liubaolin@xxxxxxxxxx>
> > >
> > > Thanks,
> > > Baolin.
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > >        if (unlikely(err)) {
> > > >                static const char *es1a = "Failed to determine if Windows is hibernated";
> > > >                static const char *es1b = "Windows is hibernated";
> > > > @@ -1581,12 +1594,25 @@ 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);
> > > > -             }
> > > > +             /*
> > > > +              * A Windows hibernation image is not a filesystem error, so
> > > > +              * this is a safety interlock rather than something the
> > > > +              * errors= policy may downgrade.  The super block is already
> > > > +              * read-only here: the temporary flag taken for the check
> > > > +              * above is not restored when the check failed.
> > > > +              */
> > > > +             ntfs_error(sb, "%s.  Mounting read-only%s", es1, es2);
> > > >                NVolSetErrors(vol);
> > > > +     } else if (unlikely(temporary_ro && sb_rdonly(sb))) {
> > > > +             static const char *es1 = "Errors were recorded during mount";
> > > > +             static const char *es2 = ".  Run chkdsk.";
> > > > +
> > > > +             /*
> > > > +              * Errors were recorded during the check or earlier, e.g. when
> > > > +              * loading the LogFile.  Stay read-only, like ntfs_reconfigure()
> > > > +              * does for volumes with recorded errors.
> > > > +              */
> > > > +             ntfs_error(sb, "%s.  Mounting read-only%s", es1, es2);
> > > >        }
> > > >
> > > >        /* If (still) a read-write mount, empty the logfile. */
> >
> >
> > --
> > Thanks,
> > Hyunchul