linux-kernel - Re: [PATCH 08/12] KVM: nSVM: Use KVM-governed feature framework to track "vVM{SAVE,LOAD} enabled"

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: <ZJ22Wts4WKKD19bN@google.com>
Date:   Thu, 29 Jun 2023 09:50:34 -0700
From:   Sean Christopherson <seanjc@...gle.com>
To:     Yu Zhang <yu.c.zhang@...ux.intel.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, Maxim Levitsky <mlevitsk@...hat.com>
Subject: Re: [PATCH 08/12] KVM: nSVM: Use KVM-governed feature framework to
 track "vVM{SAVE,LOAD} enabled"

On Wed, Feb 22, 2023, Sean Christopherson wrote:
> +Maxim
> 
> On Wed, Feb 22, 2023, Yu Zhang wrote:
> I'll opportunistically massage the comment to make it more explicit about why
> VMLOAD needs to be intercepted.
>  
> That said, clearing the bits for this seems wrong.  That would corrupt the MSRs
> for 64-bit Intel guests.  The "target" of the fix was 32-bit L2s, i.e. I doubt
> anything would notice.
> 
>     This patch fixes nested migration of 32 bit nested guests, that was
>     broken because incorrect cached values of SYSENTER msrs were stored in
>     the migration stream if L1 changed these msrs with
>     vmload prior to L2 entry.

Aha!  Finally figured out what this code is doing.  KVM intercepts VMLOAD so that
KVM can correctly model the VMLOAD behavior of dropping bits 63:32, i.e. to clear
svm->sysenter_eip_hi and svm->sysenter_esp_hi.

So the code is correct.  I'll add this comment:

	/*
	 * Intercept VMLOAD if the vCPU mode is Intel in order to emulate that
	 * VMLOAD drops bits 63:32 of SYSENTER (ignoring the fact that exposing
	 * SVM on Intel is bonkers and extremely unlikely to work).
	 */