On Thu, 2022-03-03 at 16:51 +0000, Sean Christopherson wrote:
> On Wed, Mar 02, 2022, Maxim Levitsky wrote:
> > When APIC state is loading while APIC is in *x2apic* mode it does enforce that
> > value in this 0x20 offset is initial apic id if KVM_CAP_X2APIC_API.
> >
> > I think that it is fair to also enforce this when KVM_CAP_X2APIC_API is not used,
> > especially if we make apic id read-only.
>
> I don't disagree in principle. But, (a) this loophole as existing for nearly 6
> years, (b) closing the loophole could break userspace, (c) false positive are
> possible due to truncation, and (d) KVM gains nothing meaningful by closing the
> loophole.
>
> (d) changes when we add a knob to make xAPIC ID read-only, but we can simply
> require userspace to enable KVM_CAP_X2APIC_API (or force it). That approach
> avoids (c) by eliminating truncation, and avoids (b) by virtue of being opt-in.
>
(a) - doesn't matter.
(b) - if userspace wants to have non default apic id with x2apic mode,
which (*)can't even really be set from the guest - this is ridiculous.
(*) Yes I know that in *theory* user can change apic id in xapic mode
and then switch to x2apic mode - but I really doubt that KVM
would even honor this - there are already places which assume
that this is not the case. In fact it would be nice to audit KVM
on what happens when userspace does this, there might be a nice
CVE somewhere....
(c) - without KVM_CAP_X2APIC_API, literally just call to KVM_GET_LAPIC/KVM_SET_LAPIC
will truncate x2apic id if > 255 regardless of my patch - literally this cap
was added to avoid this.
What we should do is to avoid creating cpu with vcpu_id > 256 when this cap is not set…
(d) - doesn't matter - again we are talking about x2apic mode in which apic id is read only.
Don’t get me wrong - I understand your concerns about this, but I hope that you
also understand mine - I still think that you just don't understand me.
Best regards,
Maxim Levitsky
