From 14d6cb4436f19d0500e98179c3215517f1a77b08 Mon Sep 17 00:00:00 2001
From: Andrei Vagin <avagin@google.com>
Date: Mon, 9 Aug 2021 12:30:01 -0700
Subject: 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(-)
(limited to 'pkg/sentry/platform')
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.
--
cgit v1.2.3