linux-kernel - Re: [PATCH 1/2] x86/pat: fix x86_has_pat

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f6359e71-5516-5b04-ca35-6a4870456cec@leemhuis.info>
Date:   Mon, 20 Jun 2022 08:15:45 +0200
From:   Thorsten Leemhuis <regressions@...mhuis.info>
To:     Juergen Gross <jgross@...e.com>, xen-devel@...ts.xenproject.org,
        x86@...nel.org, linux-kernel@...r.kernel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Borislav Petkov <bp@...en8.de>
Cc:     jbeulich@...e.com, Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH 1/2] x86/pat: fix x86_has_pat_wp()

On 20.06.22 07:30, Juergen Gross wrote:
> On 20.06.22 07:22, Thorsten Leemhuis wrote:
>> On 14.06.22 17:09, Juergen Gross wrote:
>>> On 03.05.22 15:22, Juergen Gross wrote:
>>>> x86_has_pat_wp() is using a wrong test, as it relies on the normal
>>>> PAT configuration used by the kernel. In case the PAT MSR has been
>>>> setup by another entity (e.g. BIOS or Xen hypervisor) it might return
>>>> false even if the PAT configuration is allowing WP mappings.
>>>>
>>>> Fixes: 1f6f655e01ad ("x86/mm: Add a x86_has_pat_wp() helper")
>>>> Signed-off-by: Juergen Gross <jgross@...e.com>
>>>> ---
>>>>    arch/x86/mm/init.c | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
>>>> index d8cfce221275..71e182ebced3 100644
>>>> --- a/arch/x86/mm/init.c
>>>> +++ b/arch/x86/mm/init.c
>>>> @@ -80,7 +80,8 @@ static uint8_t __pte2cachemode_tbl[8] = {
>>>>    /* Check that the write-protect PAT entry is set for
>>>> write-protect */
>>>>    bool x86_has_pat_wp(void)
>>>>    {
>>>> -    return __pte2cachemode_tbl[_PAGE_CACHE_MODE_WP] ==
>>>> _PAGE_CACHE_MODE_WP;
>>>> +    return
>>>> __pte2cachemode_tbl[__cachemode2pte_tbl[_PAGE_CACHE_MODE_WP]] ==
>>>> +           _PAGE_CACHE_MODE_WP;
>>>>    }
>>>>      enum page_cache_mode pgprot2cachemode(pgprot_t pgprot)
>>>
>>> x86 maintainers, please consider taking this patch, as it is fixing
>>> a real bug. Patch 2 of this series can be dropped IMO.
>>
>> Juergen, can you help me out here please. Patch 2 afaics was supposed to
>> fix this regression I'm tracking:
>> https://lore.kernel.org/regressions/YnHK1Z3o99eMXsVK@mail-itl/
> No, patch 2 wasn't covering all needed cases.

Ahh, happens. Thx for the info.

>> Is Patch 1 alone enough to fix it? Or is there a different fix for it?
> Patch 1 is fixing a different issue (it is lacking any maintainer
> feedback, though).
> 
> This patch of Jan should do the job, but it seems to be stuck, too:
> https://lore.kernel.org/lkml/9385fa60-fa5d-f559-a137-6608408f88b0@suse.com/

Ahh. Fun fact: that was on my list of things to prod, too.

>> Or is there some other solution to finally fix that regressions that
>> ideally should have been fixed weeks ago already?
> 
> I agree it should have been fixed quite some time now, but the x86
> maintainers don't seem to be interested in those stuck patches. :-(
> 
> Maybe I should take a different approach:
> 
> x86 maintainers, please speak up if you NAK (or Ack) any of above two
> patches.
> In case you don't NAK or take the patches, I'm inclined to carry them via
> the Xen tree to get the issues fixed.

Yeah, I'd be really glad if we could find a solution for this situation
and get it finally fixed in mainline and backported to stable.

Ciao, Thorsten