[PATCH] hw/vfio-user: wait for proxy close correctly

[PATCH] hw/vfio-user: wait for proxy close correctly

[John Levon]

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.

Signed-off-by: John Levon <john.levon@nutanix.com>
---
 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);
-- 
2.43.0

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

[Mark Cave-Ayland]

On 10/07/2025 16:47, John Levon wrote:

(added Cedric)

> 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.
> 
> Signed-off-by: John Levon <john.levon@nutanix.com>
> ---
>   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);

It think it is worth mentioning the function rename in the commit 
message, otherwise looks good to me:

Reviewed-by: Mark Cave-Ayland <markcaveayland@nutanix.com>


ATB,

Mark.

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

[Cédric Le Goater]

On 7/11/25 13:38, Mark Cave-Ayland wrote:
> On 10/07/2025 16:47, John Levon wrote:
> 
> (added Cedric)

ah. Thanks Mark.

This reminds me that we should have maintainers/reviewers
that can send PRs for the vfio-user component.

John,

Could you please send a patch adding me and Mark may be ?

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

Please prefer :

   Resolves: Coverity CID 1611805

>>
>> 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.
>>
>> Signed-off-by: John Levon <john.levon@nutanix.com>
>> ---
>>   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);
> 
> It think it is worth mentioning the function rename in the commit message, otherwise looks good to me:
> 
> Reviewed-by: Mark Cave-Ayland <markcaveayland@nutanix.com>



Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.

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

[Mark Cave-Ayland]

On 15/07/2025 06:49, Cédric Le Goater wrote:

> On 7/11/25 13:38, Mark Cave-Ayland wrote:
>> On 10/07/2025 16:47, John Levon wrote:
>>
>> (added Cedric)
> 
> ah. Thanks Mark.
> 
> This reminds me that we should have maintainers/reviewers
> that can send PRs for the vfio-user component.
> 
> John,
> 
> Could you please send a patch adding me and Mark may be ?

I'm still coming up to speed with the internals of QEMU's vfio 
implementation, so this might be a bit premature. But I'd like to think 
I will get there soon :)

BTW I have a series here that fixes up some of the QOM issues with vfio, 
but I'm guessing it's a little too close to freeze for this?


ATB,

Mark.

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

[Cédric Le Goater]

> BTW I have a series here that fixes up some of the QOM issues with vfio, but I'm guessing it's a little too close to freeze for this?

Please send and we'll see.

Thanks,

C.