Re: [PATCH] KVM: VMX: Forbid userspace MSR filters for x2APIC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> Am 19.10.2020 um 19:08 schrieb Paolo Bonzini <pbonzini@xxxxxxxxxx>:
> 
> Allowing userspace to intercept reads to x2APIC MSRs when APICV is
> fully enabled for the guest simply can't work.   But more in general,
> if the LAPIC is in-kernel, allowing accessed by userspace would be very
> confusing.  If userspace wants to intercept x2APIC MSRs, then it should
> first disable in-kernel APIC.
> 
> We could in principle allow userspace to intercept reads and writes to TPR,
> and writes to EOI and SELF_IPI, but while that could be made it work, it
> would still be silly.
> 
> Cc: Alexander Graf <graf@xxxxxxxxxx>
> Cc: Aaron Lewis <aaronlewis@xxxxxxxxxx>
> Cc: Peter Xu <peterx@xxxxxxxxxx>
> Cc: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
> arch/x86/kvm/x86.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index c4015a43cc8a..0faf733538f4 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5246,6 +5246,13 @@ static int kvm_add_msr_filter(struct kvm *kvm, struct kvm_msr_filter_range *user
>        return r;
> }
> 
> +static bool range_overlaps_x2apic(struct kvm_msr_filter_range *range)
> +{
> +       u32 start = range->base;
> +       u32 end = start + range->nmsrs;
> +       return start <= 0x8ff && end > 0x800;
> +}
> +
> static int kvm_vm_ioctl_set_msr_filter(struct kvm *kvm, void __user *argp)
> {
>        struct kvm_msr_filter __user *user_msr_filter = argp;
> @@ -5257,6 +5264,16 @@ static int kvm_vm_ioctl_set_msr_filter(struct kvm *kvm, void __user *argp)
>        if (copy_from_user(&filter, user_msr_filter, sizeof(filter)))
>                return -EFAULT;
> 
> +       /*
> +        * In principle it would be possible to trap x2apic ranges
> +        * if !lapic_in_kernel.  This however would be complicated
> +        * because KVM_X86_SET_MSR_FILTER can be called before
> +        * KVM_CREATE_IRQCHIP or KVM_ENABLE_CAP.
> +        */
> +       for (i = 0; i < ARRAY_SIZE(filter.ranges); i++)
> +               if (range_overlaps_x2apic(&filter.ranges[i]))
> +                       return -EINVAL;

What if the default action of the filter is to "deny"? Then only an MSR filter to allow access to x2apic MSRs would make the full filtering logic adhere to the constraints, no?

Also, this really deserves a comment in the API documentation :).

In fact, I think a pure comment in documentation is enough. Just make x2apic && filter on them "undefined behavior".


Alex

> +
>        kvm_clear_msr_filter(kvm);
> 
>        default_allow = !(filter.flags & KVM_MSR_FILTER_DEFAULT_DENY);
> --
> 2.26.2
> 



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879







[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux