2013-12-12 21:36+0100, Paolo Bonzini:
> From: Gleb Natapov <gleb@xxxxxxxxxx>
>
> A guest can cause a BUG_ON() leading to a host kernel crash.
> When the guest writes to the ICR to request an IPI, while in x2apic
> mode the following things happen, the destination is read from
> ICR2, which is a register that the guest can control.
>
> kvm_irq_delivery_to_apic_fast uses the high 16 bits of ICR2 as the
> cluster id. A BUG_ON is triggered, which is a protection against
> accessing map->logical_map with an out-of-bounds access and manages
> to avoid that anything really unsafe occurs.
>
> The logic in the code is correct from real HW point of view. The problem
> is that KVM supports only one cluster with ID 0 in clustered mode, but
> the code that has the bug does not take this into account.
The more I read about x2apic, the more confused I am ...
- How was the cluster x2apic enabled?
Linux won't enable cluster x2apic without interrupt remapping and I
had no idea we were allowed to do it.
- A hardware test-suite found this?
This bug can only be hit when the destination cpu is > 256, so the
request itself is buggy -- we don't support that many in kvm and it
would crash when initializing the vcpus if we did.
=> It looks like we should just ignore the ipi, because we have no
vcpus in that cluster.
- Where does the 'only one supported cluster' come from?
I only see we use 'struct kvm_lapic *logical_map[16][16];', which
supports 16 clusters of 16 apics = first 256 vcpus, so if we map
everything to logical_map[0][0:15], we would not work correctly in
the cluster x2apic, with > 16 vcpus.
Thanks.
> Reported-by: Lars Bull <larsbull@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Gleb Natapov <gleb@xxxxxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
> arch/x86/kvm/lapic.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index b8bec45c1610..801dc3fd66e1 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -143,6 +143,8 @@ static inline int kvm_apic_id(struct kvm_lapic *apic)
> return (kvm_apic_get_reg(apic, APIC_ID) >> 24) & 0xff;
> }
>
> +#define KMV_X2APIC_CID_BITS 0
> +
> static void recalculate_apic_map(struct kvm *kvm)
> {
> struct kvm_apic_map *new, *old = NULL;
> @@ -180,7 +182,8 @@ static void recalculate_apic_map(struct kvm *kvm)
> if (apic_x2apic_mode(apic)) {
> new->ldr_bits = 32;
> new->cid_shift = 16;
> - new->cid_mask = new->lid_mask = 0xffff;
> + new->cid_mask = (1 << KMV_X2APIC_CID_BITS) - 1;
> + new->lid_mask = 0xffff;
> } else if (kvm_apic_sw_enabled(apic) &&
> !new->cid_mask /* flat mode */ &&
> kvm_apic_get_reg(apic, APIC_DFR) == APIC_DFR_CLUSTER) {
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html