linux-kernel - Re: [PATCH 0/6] KVM: Fix bugs in vCPUs xarray usage

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: <20241029141802.GA4691@willie-the-truck>
Date: Tue, 29 Oct 2024 14:18:03 +0000
From: Will Deacon <will@...nel.org>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org, Michal Luczaj <mhal@...x.co>,
	Alexander Potapenko <glider@...gle.com>,
	Marc Zyngier <maz@...nel.org>,
	Oliver Upton <oliver.upton@...ux.dev>
Subject: Re: [PATCH 0/6] KVM: Fix bugs in vCPUs xarray usage

On Wed, Oct 09, 2024 at 08:04:49AM -0700, Sean Christopherson wrote:
> This series stems from Will's observation[*] that kvm_vm_ioctl_create_vcpu()'s
> handling of xa_store() failure when inserting into vcpu_array is technically
> broken, although in practice it's impossible for xa_store() to fail.
> 
> After much back and forth and staring, I realized that commit afb2acb2e3a3
> ("KVM: Fix vcpu_array[0] races") papered over underlying bugs in
> kvm_get_vcpu() and kvm_for_each_vcpu().  The core problem is that KVM
> allowed other tasks to see vCPU0 while online_vcpus==0, and thus trying
> to gracefully error out of vCPU creation led to use-after-free failures.
> 
> So, rather than trying to solve the unsolvable problem for an error path
> that should be impossible to hit, fix the underlying issue and ensure that
> vcpu_array[0] is accessed if and only if online_vcpus is non-zero.
> 
> Patch 3 fixes a race Michal identified when we were trying to figure out
> how to handle the xa_store() mess.
> 
> Patch 4 reverts afb2acb2e3a3.
> 
> Patches 5 and 6 are tangentially related cleanups.

Thanks, Sean. For the series:

Acked-by: Will Deacon <will@...nel.org>

I sympathise a little with Paolo on patch 4, but at the end of the day
it's a revert and I think that the code is better for it, even if the
whole scenario is messy.

Will