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

[Adhemerval Zanella Netto]

On 03/01/25 12:54, Florian Weimer wrote:
> This eliminates a TCB layout dependency from the Chromium sandbox.
> (Chromium checks the TID in a private field in a recursive mutex
> and only tries to patch the TCB if the TID found there is different
> from the value it obtains via gettid.)
> 
> Tested on x86_64-linux-gnu.  Confirmed to un-break Google Chrome.
> 
> ---
>  sysdeps/unix/sysv/linux/x86_64/clone.S | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> 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.

> +
>  /* 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?

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.

>  	/* Set up arguments for the function call.  */
>  	popq	%rax		/* Function to call.  */
>  	popq	%rdi		/* Argument.  */
> 
> base-commit: cc74583f23657515b1d09d0765032422af71de52
>