linux-kernel - Re: [PATCH] KVM: SVM: don't apply host security mitigations to the guest

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] [day] [month] [year] [list]

Message-ID: <CALMp9eRnMNkmSezPDDb_Pi8O_AG4rmdvy+pik36zMkCnSkzhhg@mail.gmail.com>
Date:   Thu, 29 Sep 2022 10:36:40 -0700
From:   Jim Mattson <jmattson@...gle.com>
To:     Tom Lendacky <thomas.lendacky@....com>
Cc:     Fares Mehanna <faresx@...zon.de>,
        Benjamin Serebrin <serebrin@...zon.com>,
        Filippo Sironi <sironi@...zon.de>,
        Sean Christopherson <seanjc@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>, Babu Moger <babu.moger@....com>,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: SVM: don't apply host security mitigations to the guest

On Thu, Sep 29, 2022 at 6:47 AM Tom Lendacky <thomas.lendacky@....com> wrote:
>
> On 9/12/22 06:58, Fares Mehanna wrote:
> > Support of virtual SPEC_CTRL caused a new behavior in KVM which made host
> > security mitigations applying by default to the guests.
>
> Maybe expand on this to say that the effective mitigation is the host
> SPEC_CTRL value or'd with guest SPEC_CTRL value.
>
> >
> > We noticed a regression after applying the patch, primarily because of the
> > enablement of SSBD on AMD Milan.
> >
> > This patch keeps the new behavior of applying host security mitigations to
> > the guests, but adds an option to disable it so the guests are free to pick
> > their own security mitigations.
> >
> > Fixes: d00b99c514b3 ("KVM: SVM: Add support for Virtual SPEC_CTRL")
> > Signed-off-by: Fares Mehanna <faresx@...zon.de>
> > Reviewed-by: Benjamin Serebrin <serebrin@...zon.com>
> > Reviewed-by: Filippo Sironi <sironi@...zon.de>
> > ---
> >   arch/x86/kvm/svm/svm.c | 12 +++++++++++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index f3813dbacb9f..c6ea24685301 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -225,6 +225,10 @@ module_param(avic, bool, 0444);
> >   bool __read_mostly dump_invalid_vmcb;
> >   module_param(dump_invalid_vmcb, bool, 0644);
> >
> > +/* enable/disable applying host security mitigations on the guest */
> > +static bool host_mitigations_on_guest = true;
> > +module_param(host_mitigations_on_guest, bool, 0444);
> > +
> >
> >   bool intercept_smi = true;
> >   module_param(intercept_smi, bool, 0444);
> > @@ -4000,6 +4004,12 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
> >        */
> >       if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL))
> >               x86_spec_ctrl_set_guest(svm->spec_ctrl, svm->virt_spec_ctrl);
> > +     else if (!host_mitigations_on_guest)
> > +             /*
> > +              * Clear the host MSR before vm-enter to avoid applying host
> > +              * security mitigations to the guest.
> > +              */
> > +             x86_spec_ctrl_set_guest(0, 0);
>
> If X86_FEATURE_V_SPEC_CTRL is active, won't svm->spec_ctrl and
> svm->virt_spec_ctrl always be zero, in which case you can do the if
> statement similar to the below one? Maybe just add a comment that those
> values will be zero in the case of X86_FEATURE_V_SPEC_CTRL, thus
> eliminating the host security mitigation effect.
>
> Thanks,
> Tom
>
> >
> >       svm_vcpu_enter_exit(vcpu);
> >
> > @@ -4025,7 +4035,7 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
> >       if (!sev_es_guest(vcpu->kvm))
> >               reload_tss(vcpu);
> >
> > -     if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL))
> > +     if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL) || !host_mitigations_on_guest)
> >               x86_spec_ctrl_restore_host(svm->spec_ctrl, svm->virt_spec_ctrl);

This is much too late to restore the host's value of IA32_SPEC_CTRL on
hosts that are using RETBLEED_MITIGATION_UNRET. The host's
IA32_SPEC_CTRL value should be restored prior to the UNTRAIN_RET in
vmenter.S.

> >       if (!sev_es_guest(vcpu->kvm)) {