linux-kernel - RE: [PATCH v2 7/8] x86/mm: only check uniform after calling mtrr_type

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 for Android: free password hash cracker in your pocket

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

Message-ID: <BYAPR21MB16882DD5A99CB365F2BD22A6D7DD9@BYAPR21MB1688.namprd21.prod.outlook.com>
Date:   Mon, 13 Feb 2023 01:08:08 +0000
From:   "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To:     Juergen Gross <jgross@...e.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>
CC:     "lists@...dbynature.de" <lists@...dbynature.de>,
        "torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>
Subject: RE: [PATCH v2 7/8] x86/mm: only check uniform after calling
 mtrr_type_lookup()

From: Juergen Gross <jgross@...e.com> Sent: Wednesday, February 8, 2023 11:22 PM
>
> Today pud_set_huge() and pmd_set_huge() test for the MTRR type to be
> WB or INVALID after calling mtrr_type_lookup(). Those tests can be
> dropped, as the only reason to not use a large mapping would be
> uniform being 0. Any MTRR type can be accepted as long as it applies
> to the whole memory range covered by the mapping, as the alternative
> would only be to map the same region with smaller pages instead using
> the same PAT type as for the large mapping.
> 
> Suggested-by: Linus Torvalds <torvalds@...ux-foundation.org>
> Signed-off-by: Juergen Gross <jgross@...e.com>
> ---
>  arch/x86/mm/pgtable.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
> index e4f499eb0f29..7b9c5443d176 100644
> --- a/arch/x86/mm/pgtable.c
> +++ b/arch/x86/mm/pgtable.c
> @@ -721,8 +721,7 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
>  	u8 mtrr, uniform;
> 
>  	mtrr = mtrr_type_lookup(addr, addr + PUD_SIZE, &uniform);
> -	if ((mtrr != MTRR_TYPE_INVALID) && (!uniform) &&
> -	    (mtrr != MTRR_TYPE_WRBACK))
> +	if (!uniform)
>  		return 0;
> 
>  	/* Bail out if we are we on a populated non-leaf entry: */
> @@ -748,8 +747,7 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot)
>  	u8 mtrr, uniform;
> 
>  	mtrr = mtrr_type_lookup(addr, addr + PMD_SIZE, &uniform);
> -	if ((mtrr != MTRR_TYPE_INVALID) && (!uniform) &&
> -	    (mtrr != MTRR_TYPE_WRBACK)) {
> +	if (!uniform) {
>  		pr_warn_once("%s: Cannot satisfy [mem %#010llx-%#010llx] with a huge-page mapping due to MTRR override.\n",
>  			     __func__, addr, addr + PMD_SIZE);

I'm seeing this warning trigger in a normal Hyper-V guest (i.e., *not* an
SEV-SNP Confidential VM).  The original filtering here based on
MTRR_TYPE_WRBACK appears to be hiding a bug in mtrr_type_lookup_variable()
where it incorrectly thinks an address range matches two different variable
MTRRs, and hence clears "uniform".

Here are the variable MTRRs in the normal Hyper-V guest with 32 GiBytes
of memory:

[    0.043592] MTRR variable ranges enabled:
[    0.048308]   0 base 000000000000 mask FFFF00000000 write-back
[    0.057450]   1 base 000100000000 mask FFF000000000 write-back
[    0.063972]   2 disabled
[    0.066755]   3 disabled
[    0.070024]   4 disabled
[    0.072856]   5 disabled
[    0.076112]   6 disabled
[    0.078760]   7 disabled

Variable MTRR #0 covers addresses up to 4 GiByte, while #1 covers
4 GiByte to 64 GiByte.   But in mtrr_type_lookup_variable(), address
range 0xF8000000 to 0xF81FFFFF is matching both MTRRs, when it
should be matching just #0.

The problem looks to be this code in mtrr_type_lookup_variable():

              if ((start & mask) != (base & mask))
                        continue;

If the mask bits of start and base are different, then the
MTRR doesn't match, and the continue statement should be
executed.  That's correct.  But if the mask bits are the same,
that's not sufficient for the MTRR to match.  If the end
address is less than base, the MTRR doesn't match, and
the continue statement should still be executed, which
isn't happening.

But somebody please check my thinking. :-)

Michael