has_waiters() is testing a reversed condition. The logic is that
has_waiters() must return true if a qemu_co_mutex_lock_slowpath()
happened:
qemu_co_mutex_unlock qemu_co_mutex_lock_slowpath
------------------------- -------------------------------
set handoff push to from_push
memory barrier memory barrier
check has_waiters() check handoff
which requires it to return true if from_push (or to_pop from a previous
call) are *not* empty.
This was unlikely to cause trouble because it can only happen when the
same CoMutex is used across multiple threads, but it is nevertheless
completely wrong. The bug would show up as either a NULL-pointer
dereference inside qemu_co_mutex_lock_slowpath(), or a missed wait in
qemu_co_mutex_unlock().
Reported-by: Siteshwar Vashisht <svashisht@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/qemu-coroutine-lock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index fac91582b5f..c82ee754beb 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -173,7 +173,7 @@ static CoWaitRecord *pop_waiter(CoMutex *mutex)
static bool has_waiters(CoMutex *mutex)
{
- return QSLIST_EMPTY(&mutex->to_pop) || QSLIST_EMPTY(&mutex->from_push);
+ return !QSLIST_EMPTY(&mutex->to_pop) || !QSLIST_EMPTY(&mutex->from_push);
}
void qemu_co_mutex_init(CoMutex *mutex)
--
2.53.0