[PATCH] linux-user: Use QemuSemaphore to wait for clone child thread

Peter Maydell posted 1 patch 1 day, 4 hours ago
linux-user/syscall.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
[PATCH] linux-user: Use QemuSemaphore to wait for clone child thread
Posted by Peter Maydell 1 day, 4 hours ago
In do_fork() and clone_func() we currently use a pthread_mutex_t and
a pthread_cond_t to allow the parent thread to wait until the child
thread has started and e.g.  set fields like info->tid, before the
parent reads those fields and otherwise proceeds.

Coverity points out that this use of a pthread condition variable
isn't reliable: pthread_cond_wait() may wake up spuriously, so the
idiom for "wait on a condition" should be "loop around calling
pthread_cond_wait() until a predicate that was updated by the caller
of pthread_cond_broadcast() is true".

This is quite old code; rather than fixing up its use of raw pthread
condition variables, we can use a QemuSemaphore.  This gives us the
"thread A waits until thread B signals it" semantics we want.  (Under
the hood it is implemented with a mutex and condition variable.)

Cc: qemu-stable@nongnu.org
Resolves: CID 1527417
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
cc stable because in theory this could result in the parent thread
waking up too early and weird stuff happening; however my guess is
that in practice with one waiter and one signaller that the
"spurious wakeup" case won't happen. At any rate this code has
been like this for a very long time.

Disclaimer: tested with "make check" only, but that does include
various linux-user tests that use multiple threads.
---
 linux-user/syscall.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 0380691a8b1..9d85a193efe 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6932,8 +6932,7 @@ static abi_long do_map_shadow_stack(CPUArchState *env, abi_ulong addr,
 static pthread_mutex_t clone_lock = PTHREAD_MUTEX_INITIALIZER;
 typedef struct {
     CPUArchState *env;
-    pthread_mutex_t mutex;
-    pthread_cond_t cond;
+    QemuSemaphore sem;
     pthread_t thread;
     uint32_t tid;
     abi_ulong child_tidptr;
@@ -6964,9 +6963,7 @@ static void *clone_func(void *arg)
     /* Enable signals.  */
     sigprocmask(SIG_SETMASK, &info->sigmask, NULL);
     /* Signal to the parent that we're ready.  */
-    pthread_mutex_lock(&info->mutex);
-    pthread_cond_broadcast(&info->cond);
-    pthread_mutex_unlock(&info->mutex);
+    qemu_sem_post(&info->sem);
     /* Wait until the parent has finished initializing the tls state.  */
     pthread_mutex_lock(&clone_lock);
     pthread_mutex_unlock(&clone_lock);
@@ -7071,9 +7068,7 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
         }
 
         memset(&info, 0, sizeof(info));
-        pthread_mutex_init(&info.mutex, NULL);
-        pthread_mutex_lock(&info.mutex);
-        pthread_cond_init(&info.cond, NULL);
+        qemu_sem_init(&info.sem, 0);
         info.env = new_env;
         if (flags & CLONE_CHILD_SETTID) {
             info.child_tidptr = child_tidptr;
@@ -7097,7 +7092,7 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
         pthread_attr_destroy(&attr);
         if (ret == 0) {
             /* Wait for the child to initialize.  */
-            pthread_cond_wait(&info.cond, &info.mutex);
+            qemu_sem_wait(&info.sem);
             ret = info.tid;
         } else {
             errno = ret;
@@ -7111,9 +7106,7 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
 #endif
             g_free(ts);
         }
-        pthread_mutex_unlock(&info.mutex);
-        pthread_cond_destroy(&info.cond);
-        pthread_mutex_destroy(&info.mutex);
+        qemu_sem_destroy(&info.sem);
         pthread_mutex_unlock(&clone_lock);
     } else {
         /* if no CLONE_VM, we consider it is a fork */
-- 
2.43.0
Re: [PATCH] linux-user: Use QemuSemaphore to wait for clone child thread
Posted by Pierrick Bouvier 1 day ago
On 10/22/2026 9:37 AM, Peter Maydell wrote:
> In do_fork() and clone_func() we currently use a pthread_mutex_t and
> a pthread_cond_t to allow the parent thread to wait until the child
> thread has started and e.g.  set fields like info->tid, before the
> parent reads those fields and otherwise proceeds.
> 
> Coverity points out that this use of a pthread condition variable
> isn't reliable: pthread_cond_wait() may wake up spuriously, so the
> idiom for "wait on a condition" should be "loop around calling
> pthread_cond_wait() until a predicate that was updated by the caller
> of pthread_cond_broadcast() is true".
> 
> This is quite old code; rather than fixing up its use of raw pthread
> condition variables, we can use a QemuSemaphore.  This gives us the
> "thread A waits until thread B signals it" semantics we want.  (Under
> the hood it is implemented with a mutex and condition variable.)
> 
> Cc: qemu-stable@nongnu.org
> Resolves: CID 1527417
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> cc stable because in theory this could result in the parent thread
> waking up too early and weird stuff happening; however my guess is
> that in practice with one waiter and one signaller that the
> "spurious wakeup" case won't happen. At any rate this code has
> been like this for a very long time.
> 
> Disclaimer: tested with "make check" only, but that does include
> various linux-user tests that use multiple threads.
> ---
>  linux-user/syscall.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 

LGTM,
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>