linux-user: Fix CLONE_PARENT_SETTID when using fork-like clone

[PATCH] linux-user: Fix CLONE_PARENT_SETTID when using fork-like clone

Posted by Helge Deller 1 month ago

From: Helge Deller <deller@gmx.de>

The CLONE_PARENT_SETTID option requires the implementation to store the
child thread ID at the location pointed to by parent_tid in the parent's
memory.

Fix our implementation and move the code from the client side (where
fork returned 0), to the parent side and store the return value from the
fork call (which is the client TID) in the parent_tid pointer.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3340
Signed-off-by: Helge Deller <deller@gmx.de>
---
 linux-user/syscall.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d0390e04b4..910a4da0a6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7050,8 +7050,6 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
                the child process gets its own copy of the lock.  */
             if (flags & CLONE_CHILD_SETTID)
                 put_user_u32(sys_gettid(), child_tidptr);
-            if (flags & CLONE_PARENT_SETTID)
-                put_user_u32(sys_gettid(), parent_tidptr);
             ts = get_task_state(cpu);
             if (flags & CLONE_SETTLS)
                 cpu_set_tls (env, newtls);
@@ -7059,6 +7057,8 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
                 ts->child_tidptr = child_tidptr;
         } else {
             cpu_clone_regs_parent(env, flags);
+            if (flags & CLONE_PARENT_SETTID)
+                put_user_u32(ret, parent_tidptr);
             if (flags & CLONE_PIDFD) {
                 int pid_fd = 0;
 #if defined(__NR_pidfd_open) && defined(TARGET_NR_pidfd_open)
-- 
2.53.0

Re: [PATCH] linux-user: Fix CLONE_PARENT_SETTID when using fork-like clone

Posted by Michael Tokarev 3 weeks, 3 days ago

On 24.04.2026 16:28, Helge Deller wrote:
> From: Helge Deller <deller@gmx.de>
> 
> The CLONE_PARENT_SETTID option requires the implementation to store the
> child thread ID at the location pointed to by parent_tid in the parent's
> memory.
> 
> Fix our implementation and move the code from the client side (where
> fork returned 0), to the parent side and store the return value from the
> fork call (which is the client TID) in the parent_tid pointer.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3340
> Signed-off-by: Helge Deller <deller@gmx.de>
> ---
>   linux-user/syscall.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

I'm picking this one up for qemu stable series too.
Please let me know if I shouldn't.

Thanks,

/mjt

Re: [PATCH] linux-user: Fix CLONE_PARENT_SETTID when using fork-like clone

Posted by Peter Maydell 1 month ago

On Fri, 24 Apr 2026 at 14:28, Helge Deller <deller@kernel.org> wrote:
>
> From: Helge Deller <deller@gmx.de>
>
> The CLONE_PARENT_SETTID option requires the implementation to store the
> child thread ID at the location pointed to by parent_tid in the parent's
> memory.
>
> Fix our implementation and move the code from the client side (where
> fork returned 0), to the parent side and store the return value from the
> fork call (which is the client TID) in the parent_tid pointer.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3340
> Signed-off-by: Helge Deller <deller@gmx.de>
> ---
>  linux-user/syscall.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index d0390e04b4..910a4da0a6 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7050,8 +7050,6 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
>                 the child process gets its own copy of the lock.  */
>              if (flags & CLONE_CHILD_SETTID)
>                  put_user_u32(sys_gettid(), child_tidptr);
> -            if (flags & CLONE_PARENT_SETTID)
> -                put_user_u32(sys_gettid(), parent_tidptr);
>              ts = get_task_state(cpu);
>              if (flags & CLONE_SETTLS)
>                  cpu_set_tls (env, newtls);
> @@ -7059,6 +7057,8 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
>                  ts->child_tidptr = child_tidptr;
>          } else {
>              cpu_clone_regs_parent(env, flags);
> +            if (flags & CLONE_PARENT_SETTID)
> +                put_user_u32(ret, parent_tidptr);
>              if (flags & CLONE_PIDFD) {
>                  int pid_fd = 0;

The clone(2) manpage says that CLONE_PARENT_SETTID stores the
*child* thread ID -- shouldn't we be storing child_tidptr here ?

thanks
-- PMM

Re: [PATCH] linux-user: Fix CLONE_PARENT_SETTID when using fork-like clone

Posted by Helge Deller 1 month ago

Hi Peter,

On 4/24/26 15:39, Peter Maydell wrote:
> On Fri, 24 Apr 2026 at 14:28, Helge Deller <deller@kernel.org> wrote:
>>
>> From: Helge Deller <deller@gmx.de>
>>
>> The CLONE_PARENT_SETTID option requires the implementation to store the
>> child thread ID at the location pointed to by parent_tid in the parent's
>> memory.
>>
>> Fix our implementation and move the code from the client side (where
>> fork returned 0), to the parent side and store the return value from the
>> fork call (which is the client TID) in the parent_tid pointer.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3340
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> ---
>>   linux-user/syscall.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index d0390e04b4..910a4da0a6 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -7050,8 +7050,6 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
>>                  the child process gets its own copy of the lock.  */
>>               if (flags & CLONE_CHILD_SETTID)
>>                   put_user_u32(sys_gettid(), child_tidptr);
>> -            if (flags & CLONE_PARENT_SETTID)
>> -                put_user_u32(sys_gettid(), parent_tidptr);
>>               ts = get_task_state(cpu);
>>               if (flags & CLONE_SETTLS)
>>                   cpu_set_tls (env, newtls);
>> @@ -7059,6 +7057,8 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
>>                   ts->child_tidptr = child_tidptr;
>>           } else {
>>               cpu_clone_regs_parent(env, flags);
>> +            if (flags & CLONE_PARENT_SETTID)
>> +                put_user_u32(ret, parent_tidptr);
>>               if (flags & CLONE_PIDFD) {
>>                   int pid_fd = 0;
> 
> The clone(2) manpage says that CLONE_PARENT_SETTID stores the
> *child* thread ID -- shouldn't we be storing child_tidptr here ?
Sorry, but I don't get your point.
Even before we stored the TID of the child (sys_gettid())
at the location "parent_tidptr". Now we still store the TID of the child (which
is the one returned from the fork()) at "paret_tidptr".
Why are you mentioning child_tidptr?

Helge

Re: [PATCH] linux-user: Fix CLONE_PARENT_SETTID when using fork-like clone

Posted by Peter Maydell 1 month ago

On Fri, 24 Apr 2026 at 15:36, Helge Deller <deller@gmx.de> wrote:
>
> Hi Peter,
>
> On 4/24/26 15:39, Peter Maydell wrote:
> >> @@ -7059,6 +7057,8 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
> >>                   ts->child_tidptr = child_tidptr;
> >>           } else {
> >>               cpu_clone_regs_parent(env, flags);
> >> +            if (flags & CLONE_PARENT_SETTID)
> >> +                put_user_u32(ret, parent_tidptr);
> >>               if (flags & CLONE_PIDFD) {
> >>                   int pid_fd = 0;
> >
> > The clone(2) manpage says that CLONE_PARENT_SETTID stores the
> > *child* thread ID -- shouldn't we be storing child_tidptr here ?
> Sorry, but I don't get your point.
> Even before we stored the TID of the child (sys_gettid())
> at the location "parent_tidptr". Now we still store the TID of the child (which
> is the one returned from the fork()) at "paret_tidptr".
> Why are you mentioning child_tidptr?

I got confused about whether parent_tidptr was the value
or the address in this put_user function, sorry. (I think
maybe my intuition is that it ought to be a (dest, src) order
like memcpy, but it isn't.)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM