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

[liubaolin <liubaolin12138@163.com>]

在 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@gmail.com> 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@kylinos.cn>
>>>
>>> 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