[PATCH] x86_64: Reset TID in TCB on clone that does not use CLONE_VM

[Florian Weimer]

* Adhemerval Zanella Netto:

>> diff --git a/sysdeps/unix/sysv/linux/x86_64/clone.S b/sysdeps/unix/sysv/linux/x86_64/clone.S
>> index f3985e7f71..67a30405a2 100644
>> --- a/sysdeps/unix/sysv/linux/x86_64/clone.S
>> +++ b/sysdeps/unix/sysv/linux/x86_64/clone.S
>> @@ -23,6 +23,9 @@
>>  #include <bits/errno.h>
>>  #include <asm-syntax.h>
>>  
>> +/* Neither <sched.h> nor <linux/sched.h> are usable in assembler files.  */
>> +#define CLONE_VM 0x00000100
>
> You can the constant with a *.sym file, like tcb-offsets.sym for some
> ABIs.

Makes sense.

>>  /* The userland implementation is:
>>     int clone (int (*fn)(void *arg), void *child_stack, int flags,
>>  	      void *arg, pid_t *parent_tid, void *tls, pid_t *child_tid);
>> @@ -94,6 +97,15 @@ L(thread_start):
>>  	   the outermost frame obviously.  */
>>  	xorl	%ebp, %ebp
>>  
>> +	/* If a fork-style clone is performed, reset the TID in the TCB.
>> +	   Without CLONE_VM, the TID is not shared and safe to re-use.  */
>> +	test	$CLONE_VM, %edi
>> +	jnz	1f
>> +	movl	$SYS_ify(gettid), %eax
>> +	syscall
>> +	movl	%eax, %fs:TID
>> +1:
>> +
>
> Why Chrome check the TID field after call clone? I had the impression that
> once a process calls clone() directly is undefined behavior whether calls
> to pthread should succeed. Is the TID reset suffice to make pthread consistent
> is this case, or is just to make recursive mutex to work?

Chrome needs a ForkWithFlags implementation.  They currently call our
clone (without CLONE_VM and CLONE_VFORK) and longjmp out of the
callback.  This results in a TCB that doesn't have the right TID, and as
a result, recursive mutexes may not work correctly due to TID reuse
(I assume that's the bug they are trying to fix by patching the TID).

> In any case, we should make this change on all ABIs and not only on x86.
> And I think we should proper document this with a bugzilla.

I would rather add a clone_fork implementation which requires CLONE_VM
or CLONE_VFORK for proper operation.  To fix the other architectures, we
can add back the old 24-pointer padding.  Or add the TID reset for the
architectures that have a Chromium port.

I tried to reach out to the Chromium developers.  I think I'm subscribed
to the list, but I don't see the posting in the archives yet:

From: Florian Weimer <fweimer@redhat.com>
Subject: Reducing dependencies on glibc internals in
 sandbox/linux/services/namespace_sandbox.cc, ForkWithFlags
To: chromium-dev@chromium.org
Date: Fri, 03 Jan 2025 17:04:14 +0100
Message-ID: <87bjwnlw8x.fsf@oldenburg.str.redhat.com>

Thanks,
Florian