Re: [PATCH 6.6] KVM: x86: Make x2APIC ID 100% readonly

[Sasha Levin <sashal@kernel.org>]

On Thu, Dec 26, 2024 at 11:38:47AM +0800, Gavin Guo wrote:
>From: Sean Christopherson <seanjc@google.com>
>
>[ Upstream commit 4b7c3f6d04bd53f2e5b228b6821fb8f5d1ba3071 ]
>
>Ignore the userspace provided x2APIC ID when fixing up APIC state for
>KVM_SET_LAPIC, i.e. make the x2APIC fully readonly in KVM.  Commit
>a92e2543d6a8 ("KVM: x86: use hardware-compatible format for APIC ID
>register"), which added the fixup, didn't intend to allow userspace to
>modify the x2APIC ID.  In fact, that commit is when KVM first started
>treating the x2APIC ID as readonly, apparently to fix some race:
>
> static inline u32 kvm_apic_id(struct kvm_lapic *apic)
> {
>-       return (kvm_lapic_get_reg(apic, APIC_ID) >> 24) & 0xff;
>+       /* To avoid a race between apic_base and following APIC_ID update when
>+        * switching to x2apic_mode, the x2apic mode returns initial x2apic id.
>+        */
>+       if (apic_x2apic_mode(apic))
>+               return apic->vcpu->vcpu_id;
>+
>+       return kvm_lapic_get_reg(apic, APIC_ID) >> 24;
> }
>
>Furthermore, KVM doesn't support delivering interrupts to vCPUs with a
>modified x2APIC ID, but KVM *does* return the modified value on a guest
>RDMSR and for KVM_GET_LAPIC.  I.e. no remotely sane setup can actually
>work with a modified x2APIC ID.
>
>Making the x2APIC ID fully readonly fixes a WARN in KVM's optimized map
>calculation, which expects the LDR to align with the x2APIC ID.
>
>  WARNING: CPU: 2 PID: 958 at arch/x86/kvm/lapic.c:331 kvm_recalculate_apic_map+0x609/0xa00 [kvm]
>  CPU: 2 PID: 958 Comm: recalc_apic_map Not tainted 6.4.0-rc3-vanilla+ #35
>  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.16.2-1-1 04/01/2014
>  RIP: 0010:kvm_recalculate_apic_map+0x609/0xa00 [kvm]
>  Call Trace:
>   <TASK>
>   kvm_apic_set_state+0x1cf/0x5b0 [kvm]
>   kvm_arch_vcpu_ioctl+0x1806/0x2100 [kvm]
>   kvm_vcpu_ioctl+0x663/0x8a0 [kvm]
>   __x64_sys_ioctl+0xb8/0xf0
>   do_syscall_64+0x56/0x80
>   entry_SYSCALL_64_after_hwframe+0x46/0xb0
>  RIP: 0033:0x7fade8b9dd6f
>
>Unfortunately, the WARN can still trigger for other CPUs than the current
>one by racing against KVM_SET_LAPIC, so remove it completely.
>
>Reported-by: Michal Luczaj <mhal@rbox.co>
>Closes: https://lore.kernel.org/all/814baa0c-1eaa-4503-129f-059917365e80@rbox.co
>Reported-by: Haoyu Wu <haoyuwu254@gmail.com>
>Closes: https://lore.kernel.org/all/20240126161633.62529-1-haoyuwu254@gmail.com
>Reported-by: syzbot+545f1326f405db4e1c3e@syzkaller.appspotmail.com
>Closes: https://lore.kernel.org/all/000000000000c2a6b9061cbca3c3@google.com
>Signed-off-by: Sean Christopherson <seanjc@google.com>
>Message-ID: <20240802202941.344889-2-seanjc@google.com>
>Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>Signed-off-by: Gavin Guo <gavinguo@igalia.com>

As this one isn't tagged for stable, the KVM maintainers should ack the
backport before we take it.

-- 
Thanks,
Sasha