Re: [PATCH v2 3/4] hw/vfio-user: wait for proxy close correctly

[Mark Cave-Ayland]

On 15/07/2025 06:52, John Levon wrote:

Coverity reported:
CID 1611806: Concurrent data access violations (BAD_CHECK_OF_WAIT_COND)

A wait is performed without a loop. If there is a spurious wakeup, the
condition may not be satisfied.

Fix this by checking ->state for VFIO_PROXY_CLOSED in a loop.

Resolves: Coverity CID 1611806
Fixes: 0b3d881a ("vfio-user: implement message receive infrastructure")
Signed-off-by: John Levon <john.le...@nutanix.com>

Is this definitely the right patch? The v2 posted at 
https://patchew.org/QEMU/20250711124500.1611628-1-john.le...@nutanix.com/ 
contains the updated commit message mentioning the rename of the 
callback, whereas this one doesn't?

---
  hw/vfio-user/proxy.c | 10 ++++++----
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c
index c418954440..2275d3fe39 100644
--- a/hw/vfio-user/proxy.c
+++ b/hw/vfio-user/proxy.c
@@ -32,7 +32,6 @@ static void vfio_user_recycle(VFIOUserProxy *proxy, 
VFIOUserMsg *msg);
  
  static void vfio_user_recv(void *opaque);
  static void vfio_user_send(void *opaque);
-static void vfio_user_cb(void *opaque);
  
  static void vfio_user_request(void *opaque);
  
@@ -492,7 +491,7 @@ static void vfio_user_send(void *opaque)
      }
  }
  
-static void vfio_user_cb(void *opaque)
+static void vfio_user_close_cb(void *opaque)
  {
      VFIOUserProxy *proxy = opaque;
  
@@ -984,8 +983,11 @@ void vfio_user_disconnect(VFIOUserProxy *proxy)
       * handler to run after the proxy fd handlers were
       * deleted above.
       */
-    aio_bh_schedule_oneshot(proxy->ctx, vfio_user_cb, proxy);
-    qemu_cond_wait(&proxy->close_cv, &proxy->lock);
+    aio_bh_schedule_oneshot(proxy->ctx, vfio_user_close_cb, proxy);
+
+    while (proxy->state != VFIO_PROXY_CLOSED) {
+        qemu_cond_wait(&proxy->close_cv, &proxy->lock);
+    }
  
      /* we now hold the only ref to proxy */
      qemu_mutex_unlock(&proxy->lock);


ATB,

Mark.