From 14d6cb4436f19d0500e98179c3215517f1a77b08 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Mon, 9 Aug 2021 12:30:01 -0700 Subject: [PATCH] platform/kvm: fix a race condition in vCPU.unlock() Right now, it contains the code: origState := atomic.LoadUint32(&c.state) atomicbitops.AndUint32(&c.state, ^vCPUUser) The problem here is that vCPU.bounce that is called from another thread can add vCPUWaiter when origState has been read but vCPUUser isn't cleared yet. In this case, vCPU.unlock doesn't notify other threads about changes and c.bounce will be stuck in the futex_wait call. PiperOrigin-RevId: 389697411 --- pkg/sentry/platform/kvm/machine.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/sentry/platform/kvm/machine.go b/pkg/sentry/platform/kvm/machine.go index e7092a756..d67563958 100644 --- a/pkg/sentry/platform/kvm/machine.go +++ b/pkg/sentry/platform/kvm/machine.go @@ -519,15 +519,21 @@ func (c *vCPU) lock() { // //go:nosplit func (c *vCPU) unlock() { - if atomic.CompareAndSwapUint32(&c.state, vCPUUser|vCPUGuest, vCPUGuest) { + origState := atomicbitops.CompareAndSwapUint32(&c.state, vCPUUser|vCPUGuest, vCPUGuest) + if origState == vCPUUser|vCPUGuest { // Happy path: no exits are forced, and we can continue // executing on our merry way with a single atomic access. return } // Clear the lock. - origState := atomic.LoadUint32(&c.state) - atomicbitops.AndUint32(&c.state, ^vCPUUser) + for { + state := atomicbitops.CompareAndSwapUint32(&c.state, origState, origState&^vCPUUser) + if state == origState { + break + } + origState = state + } switch origState { case vCPUUser: // Normal state.