| 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
| ||
|
Message-ID: <4d9b2bd1eac524335e51253d59bfd805ea691f24.camel@infradead.org>
Date: Fri, 26 Feb 2021 10:41:48 +0000
From: David Woodhouse <dwmw2@...radead.org>
To: Paolo Bonzini <pbonzini@...hat.com>, linux-kernel@...r.kernel.org,
kvm@...r.kernel.org
Cc: Christoph Hellwig <hch@....de>
Subject: Re: [EXTERNAL] [PATCH] KVM: x86: allow compiling out the Xen
hypercall interface
On Fri, 2021-02-26 at 05:14 -0500, Paolo Bonzini wrote:
> The Xen hypercall interface adds to the attack surface of the hypervisor
> and will be used quite rarely. Allow compiling it out.
>
> Suggested-by: Christoph Hellwig <hch@....de>
> Cc: David Woodhouse <dwmw@...zon.co.uk>
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> ---
> arch/x86/kvm/Kconfig | 9 ++++++++
> arch/x86/kvm/Makefile | 3 ++-
> arch/x86/kvm/x86.c | 2 ++
> arch/x86/kvm/xen.h | 49 ++++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index 7ac592664c52..5a0d704581bd 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -103,6 +103,15 @@ config KVM_AMD_SEV
> Provides support for launching Encrypted VMs (SEV) and Encrypted VMs
> with Encrypted State (SEV-ES) on AMD processors.
>
> +config KVM_XEN
> + bool "Support for Xen hypercall interface"
> + depends on KVM && IA32_FEAT_CTL
> + help
> + Provides KVM support for the Xen shared information page and
> + for passing Xen hypercalls to userspace.
I'd be a bit less specific there. Just "support for hosting Xen HVM
guests" perhaps? It already includes basic event channel support, I'm
posted an RFC for the runstate stuff, and more event channel
acceleration is next...
> + If in doubt, say "N".
> +
> config KVM_MMU_AUDIT
> bool "Audit KVM MMU"
> depends on KVM && TRACEPOINTS
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index aeab168c5711..1b4766fe1de2 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -14,11 +14,12 @@ kvm-y += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
> $(KVM)/dirty_ring.o
> kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/async_pf.o
>
> -kvm-y += x86.o emulate.o i8259.o irq.o lapic.o xen.o \
> +kvm-y += x86.o emulate.o i8259.o irq.o lapic.o \
> i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mtrr.o \
> hyperv.o debugfs.o mmu/mmu.o mmu/page_track.o \
> mmu/spte.o
> kvm-$(CONFIG_X86_64) += mmu/tdp_iter.o mmu/tdp_mmu.o
> +kvm-$(CONFIG_KVM_XEN) += xen.o
>
> kvm-intel-y += vmx/vmx.o vmx/vmenter.o vmx/pmu_intel.o vmx/vmcs12.o \
> vmx/evmcs.o vmx/nested.o vmx/posted_intr.o
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index bfc928495bd4..9727295b7817 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8039,8 +8039,10 @@ void kvm_arch_exit(void)
> kvm_mmu_module_exit();
> free_percpu(user_return_msrs);
> kmem_cache_destroy(x86_fpu_cache);
> +#ifdef CONFIG_KVM_XEN
> static_key_deferred_flush(&kvm_xen_enabled);
> WARN_ON(static_branch_unlikely(&kvm_xen_enabled.key));
> +#endif
We could always just drop that. It's just paranoia.
> }
>
> static int __kvm_vcpu_halt(struct kvm_vcpu *vcpu, int state, int reason)
> diff --git a/arch/x86/kvm/xen.h b/arch/x86/kvm/xen.h
> index b66a921776f4..6abdbebb029b 100644
> --- a/arch/x86/kvm/xen.h
> +++ b/arch/x86/kvm/xen.h
> @@ -9,6 +9,7 @@
> #ifndef __ARCH_X86_KVM_XEN_H__
> #define __ARCH_X86_KVM_XEN_H__
>
> +#ifdef CONFIG_KVM_XEN
> #include <linux/jump_label_ratelimit.h>
>
> extern struct static_key_false_deferred kvm_xen_enabled;
> @@ -18,7 +19,6 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
> int kvm_xen_vcpu_get_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data);
> int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data);
> int kvm_xen_hvm_get_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data);
> -int kvm_xen_hypercall(struct kvm_vcpu *vcpu);
> int kvm_xen_write_hypercall_page(struct kvm_vcpu *vcpu, u64 data);
> int kvm_xen_hvm_config(struct kvm *kvm, struct kvm_xen_hvm_config *xhc);
> void kvm_xen_destroy_vm(struct kvm *kvm);
> @@ -38,6 +38,53 @@ static inline int kvm_xen_has_interrupt(struct kvm_vcpu *vcpu)
>
> return 0;
> }
> +#else
> +static inline int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int kvm_xen_vcpu_get_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int kvm_xen_hvm_get_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int kvm_xen_write_hypercall_page(struct kvm_vcpu *vcpu, u64 data)
> +{
> + return 1;
> +}
> +
> +static inline int kvm_xen_hvm_config(struct kvm *kvm, struct kvm_xen_hvm_config *xhc)
> +{
> + return -EINVAL;
> +}
I wonder if at least for the ioctls, it might be nicer to put the
#ifdef CONFIG_KVM_XEN around the callers for these? If we did it that
way we'd probably spot the fact that we need to stop advertising the
feature too... :)
Download attachment "smime.p7s" of type "application/x-pkcs7-signature" (5174 bytes)
Powered by blists - more mailing lists