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: <ZyLTRvbLq9ZTXBim@google.com>
Date: Wed, 30 Oct 2024 17:45:58 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Maxim Levitsky <mlevitsk@...hat.com>
Cc: kvm@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>, 
	Paolo Bonzini <pbonzini@...hat.com>, Ingo Molnar <mingo@...hat.com>, 
	Vitaly Kuznetsov <vkuznets@...hat.com>, linux-kernel@...r.kernel.org, 
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>
Subject: Re: [PATCH v4 3/4] KVM: x86: model canonical checks more precisely

On Fri, Sep 06, 2024, Maxim Levitsky wrote:
> As a result of a recent investigation, it was determined that x86 CPUs
> which support 5-level paging, don't always respect CR4.LA57 when doing
> canonical checks.
> 
> In particular:
> 
> 1. MSRs which contain a linear address, allow full 57-bitcanonical address
> regardless of CR4.LA57 state. For example: MSR_KERNEL_GS_BASE.
> 
> 2. All hidden segment bases and GDT/IDT bases also behave like MSRs.
> This means that full 57-bit canonical address can be loaded to them
> regardless of CR4.LA57, both using MSRS (e.g GS_BASE) and instructions
> (e.g LGDT).
> 
> 3. TLB invalidation instructions also allow the user to use full 57-bit
> address regardless of the CR4.LA57.
> 
> Finally, it must be noted that the CPU doesn't prevent the user from
> disabling 5-level paging, even when the full 57-bit canonical address is
> present in one of the registers mentioned above (e.g GDT base).
> 
> In fact, this can happen without any userspace help, when the CPU enters
> SMM mode - some MSRs, for example MSR_KERNEL_GS_BASE are left to contain
> a non-canonical address in regard to the new mode.
> 
> Since most of the affected MSRs and all segment bases can be read and
> written freely by the guest without any KVM intervention, this patch makes
> the emulator closely follow hardware behavior, which means that the
> emulator doesn't take in the account the guest CPUID support for 5-level
> paging, and only takes in the account the host CPU support.
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@...hat.com>
> ---
>  arch/x86/kvm/emulate.c       |  2 +-
>  arch/x86/kvm/mmu/mmu.c       |  2 +-
>  arch/x86/kvm/vmx/nested.c    | 22 ++++++++--------
>  arch/x86/kvm/vmx/pmu_intel.c |  2 +-
>  arch/x86/kvm/vmx/sgx.c       |  2 +-
>  arch/x86/kvm/vmx/vmx.c       |  4 +--
>  arch/x86/kvm/x86.c           |  8 +++---
>  arch/x86/kvm/x86.h           | 49 ++++++++++++++++++++++++++++++++++--
>  8 files changed, 68 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 8c8061884a019..60986f67c35a8 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -654,7 +654,7 @@ static inline bool emul_is_noncanonical_address(u64 la,
>  						struct x86_emulate_ctxt *ctxt,
>  						unsigned int flags)
>  {
> -	return !ctxt->ops->is_canonical_addr(ctxt, la, 0);
> +	return !ctxt->ops->is_canonical_addr(ctxt, la, flags);

And conversely, passing flags to ->is_canonical_addr() belongs in the patch that
adds the plumbing.  Even though flags isn't truly consumed until this patch,
passing '0' in this helper is confusing and an unnecessary risk.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ