[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20131216162242.GH21068@minantech.com>
Date: Mon, 16 Dec 2013 18:22:42 +0200
From: Gleb Natapov <gleb@...antech.com>
To: Radim Krčmář <rkrcmar@...hat.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, linux-kernel@...r.kernel.org,
kvm@...r.kernel.org, pmatouse@...hat.com, stable@...r.kernel.org,
larsbull@...gle.com
Subject: Re: [PATCH] KVM: x86: fix guest-initiated crash with x2apic
(CVE-2013-6376)
On Mon, Dec 16, 2013 at 02:31:43PM +0100, Radim Krčmář wrote:
> 2013-12-16 13:55+0100, Radim Krčmář:
> > 2013-12-16 14:16+0200, Gleb Natapov:
> > > On Mon, Dec 16, 2013 at 01:01:10PM +0100, Radim Krčmář wrote:
> > > > > > - Where does the 'only one supported cluster' come from?
> > > > > >
> > > > > "only one supported cluster" comes from 8 bit cpuid limitation of KVM's x2apic
> > > > > implementation. With 8 bit cpuid you can only address cluster 0 in logical mode.
> > > >
> > > > One x2apic cluster has 16 cpus and we generate the x2apic LDR correctly,
> > > > so 8 bit cpuid can address first 16 clusters as well.
> > > >
> > > > u32 ldr = ((id >> 4) << 16) | (1 << (id & 0xf));
> > > >
> > > Interrupt from a device cannot generate such ldr, only IPI can. Only
> > > 4 cpus in cluster zero are addressable in clustering mode by a
> > > device. Without irq remapping x2apic is a PV interface between host
> > > and guest where guest needs to know KVM implementation's limitation to
> > > use it.
> >
> > Thanks, I'll read more about devices ... still no idea how could they
> > address cluster > 15.
> >
With irq remapping device they will be able to provide 32bit apic addresses.
> > > I do not see a point in fixing problems in x2apic logical mode
> > > emulation right now since it will not make it usable, as long as
> > > there is not security problems there.
> >
> > Agreed; I wanted to know why this patch was correct, if we cared.
> >
> > > > > > 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.
> > > > > >
> > > > > Such config cannot work today because of 8 bit cpuid limitation. When the limitation
> > > > > will be removed KMV_X2APIC_CID_BITS will be set to actual number of bits we want to support.
> > > >
> > > > Even with KMV_X2APIC_CID_BITS = 4, which would allow us to support 8 bit
> > > > cpuid, we would still deliver interrupts destined for cpuid > 256 to
> > > > potentially plugged cpus.
> > > Again, KMV_X2APIC_CID_BITS = 4 will not allow us to support 8 bit cpuids
> > > unfortunately, not sure what you mean by the second part of the sentence.
> >
> > Sorry, I meant that with this change, we map all clusters to cluster 0,
> > which has two flaws:
> > - in kvm_lapic_set_base(), the order of vcpu creation determines those
>
> Gah, should have been 'recalculate_apic_map()' ...
>
> The patch would be especially surprising with a dynamic adding of vcpus.
>
> > assigned to cluster 0, and the rest is unaddressable (overwritten)
> > - we can send IPI to an unplugged high cpuid and it arives in cluster 0
It does not matter since well behaved guest is not supposed to configure apic like
that.
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists