linux-kernel - Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

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]

Date: Thu, 1 Feb 2024 10:29:00 -0800
From: Dave Hansen <dave.hansen@...el.com>
To: Paolo Bonzini <pbonzini@...hat.com>, linux-kernel@...r.kernel.org,
 kvm@...r.kernel.org
Cc: Zixi Chen <zixchen@...hat.com>, Adam Dunlap <acdunlap@...gle.com>,
 "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
 Xiaoyao Li <xiaoyao.li@...el.com>, Kai Huang <kai.huang@...el.com>,
 Dave Hansen <dave.hansen@...ux.intel.com>,
 Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...nel.org>,
 x86@...nel.org, stable@...r.kernel.org
Subject: Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or
 TME

On 1/31/24 15:09, Paolo Bonzini wrote:
> However, as noticed by Kirill, the patch I sent as v1 actually works only
> until Linux 6.6.  In Linux 6.7, commit fbf6449f84bf ("x86/sev-es: Set
> x86_virt_bits to the correct value straight away, instead of a two-phase
> approach") reorganized the initialization of c->x86_phys_bits in a way
> that broke the patch.  But even in 6.7 AMD processors, which did try to
> reduce it in this_cpu->c_early_init(c), had their x86_phys_bits value
> overwritten by get_cpu_address_sizes(), so that early_identify_cpu()
> left the wrong value in x86_phys_bits.  This probably went unnoticed
> because on AMD processors you need not apply the reduced MAXPHYADDR to
> MTRR masks.

I really wanted get_cpu_address_sizes() to be the one and only spot
where c->x86_phys_bits is established.  That way, we don't get a bunch
of code all of the place tweaking it and fighting for who "wins".

We're not there yet, but the approach in this patch moves it back in the
wrong direction because it permits the random tweaking of c->x86_phys_bits.

Could we instead do something more like the (completely untested)
attached patch?

BTW, I'm pretty sure the WARN_ON() in the patch won't actually work, but
it'd be nice to work toward a point when it does.