diff options
author | Andrei Vagin <avagin@google.com> | 2021-08-09 12:30:01 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2021-08-09 12:32:31 -0700 |
commit | 14d6cb4436f19d0500e98179c3215517f1a77b08 (patch) | |
tree | dacdececd2bd9aceb154eaa44e684920b7341ada /pkg | |
parent | 34ec00c5e775479b15ae8ad69456cef02f0a545a (diff) |
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
Diffstat (limited to 'pkg')
-rw-r--r-- | pkg/sentry/platform/kvm/machine.go | 12 |
1 files 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. |