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

[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!

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