linux-kernel - Re: [PATCH v3 1/3] KVM: x86: Bail from kvm_recalculate_phys

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZHqHHLa2G4r0aXJt@google.com>
Date:   Fri, 2 Jun 2023 17:19:40 -0700
From:   Sean Christopherson <seanjc@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, Michal Luczaj <mhal@...x.co>
Subject: Re: [PATCH v3 1/3] KVM: x86: Bail from kvm_recalculate_phys_map() if
 x2APIC ID is out-of-bounds

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@...x.co>

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

> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---