Re: [PATCH v3 1/3] KVM: x86: Bail from kvm_recalculate_phys_map() if x2APIC ID is out-of-bounds

[Sean Christopherson]

On Fri, Jun 02, 2023, Sean Christopherson wrote:
> Bail from kvm_recalculate_phys_map() and disable the optimized map if the
> target vCPU's x2APIC ID is out-of-bounds, i.e. if the vCPU was added
> and/or enabled its local APIC after the map was allocated.  This fixes an
> out-of-bounds access bug in the !x2apic_format path where KVM would write
> beyond the end of phys_map.
> 
> Check the x2APIC ID regardless of whether or not x2APIC is enabled,
> as KVM's hardcodes x2APIC ID to be the vCPU ID, i.e. it can't change, and
> the map allocation in kvm_recalculate_apic_map() doesn't check for x2APIC
> being enabled, i.e. the check won't get false postivies.
> 
> Note, this also affects the x2apic_format path, which previously just
> ignored the "x2apic_id > new->max_apic_id" case.  That too is arguably a
> bug fix, as ignoring the vCPU meant that KVM would not send interrupts to
> the vCPU until the next map recalculation.  In practice, that "bug" is
> likely benign as a newly present vCPU/APIC would immediately trigger a
> recalc.  But, there's no functional downside to disabling the map, and
> a future patch will gracefully handle the -E2BIG case by retrying instead
> of simply disabling the optimized map.
> 
> Opportunistically add a sanity check on the xAPIC ID size, along with a
> comment explaining why the xAPIC ID is guaranteed to be "good".
> 
> Reported-by: Michal Luczaj <mhal@xxxxxxx>

Fixes: 5b84b0291702 ("KVM: x86: Honor architectural behavior for aliased 8-bit APIC IDs")
Cc: stable@xxxxxxxxxxxxxxx

> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---