linux-kernel - Re: [RFC PATCH 2/2] KVM: x86: Enable guest SSP read/write interface with new uAPIs

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: <ZmelpPm5YfGifhIj@google.com>
Date: Mon, 10 Jun 2024 18:17:24 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Yang Weijiang <weijiang.yang@...el.com>
Cc: pbonzini@...hat.com, mlevitsk@...hat.com, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 2/2] KVM: x86: Enable guest SSP read/write interface
 with new uAPIs

On Thu, May 09, 2024, Yang Weijiang wrote:
> Enable guest shadow stack pointer(SSP) access interface with new uAPIs.
> CET guest SSP is HW register which has corresponding VMCS field to save
> /restore guest values when VM-{Exit,Entry} happens. KVM handles SSP as
> a synthetic MSR for userspace access.
> 
> Use a translation helper to set up mapping for SSP synthetic index and
> KVM-internal MSR index so that userspace doesn't need to take care of
> KVM's management for synthetic MSRs and avoid conflicts.
> 
> Suggested-by: Sean Christopherson <seanjc@...gle.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@...el.com>
> ---
>  arch/x86/include/uapi/asm/kvm.h |  3 +++
>  arch/x86/kvm/x86.c              |  7 +++++++
>  arch/x86/kvm/x86.h              | 10 ++++++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index ca2a47a85fa1..81c8d9ea2e58 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -420,6 +420,9 @@ struct kvm_x86_reg_id {
>  	__u16 rsvd16;
>  };
>  
> +/* KVM synthetic MSR index staring from 0 */
> +#define MSR_KVM_GUEST_SSP	0

Do we want to have "SYNTHETIC" in the name?  E.g. to try and differentiate from
KVM's paravirtual MSRs?

Hmm, but the PV MSRs are synthetic too.  Maybe it's the MSR part that's bad, e.g.
the whole point of these shenanigans is to let KVM use its internal MSR framework
without exposing those details to userspace.

So rather than, KVM_X86_REG_SYNTHETIC_MSR, what if we go with KVM_X86_REG_SYNTHETIC?
And then this becomes something like KVM_SYNTHETIC_GUEST_SSP?

Aha!  And then to prepare for a future where we add synthetic registers that
aren't routed through the MSR framework (which seems unlikely, but its trivially
easy to handle, so why not):

static int kvm_translate_synthetic_reg(struct kvm_x86_reg_id *reg)
{
	switch (reg->index) {
	case MSR_KVM_GUEST_SSP:
		reg->type = KVM_X86_REG_MSR;
		reg->index = MSR_KVM_INTERNAL_GUEST_SSP;
		break;
	default:
		return -EINVAL;
	}
	return 0;
}

and then the caller would have slightly different ordering:

        if (id->type == KVM_X86_REG_SYNTHETIC_MSR) {
                r = kvm_translate_synthetic_msr(&id->index);
                if (r)
                        break;
        }

        r = -EINVAL;
        if (id->type != KVM_X86_REG_MSR)
                break;