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 <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]> --- 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
