[PATCH] nptl: clear the whole rseq area before registration

[Michael Jeanson]

On 2025-02-14 12:47, Florian Weimer wrote:
> * Michael Jeanson:
> 
>> Due to the extensible nature of the rseq area we can't explictly
>> initialize fields that are not part of the ABI yet. It was agreed with
>> upstream that all new fields will be documented as zero initialized by
>> userspace. Future kernels configured with CONFIG_DEBUG_RSEQ will
>> validate the content of all fields during registration.
>>
>> Replace the explicit field initialization with a memset of the whole
>> rseq area which will cover fields as they are added to future kernels.
>>
>> Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
>> ---
>>  sysdeps/unix/sysv/linux/rseq-internal.h | 11 +++++------
>>  1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h b/sysdeps/unix/sysv/linux/rseq-internal.h
>> index f89e784243..d2ab4cb829 100644
>> --- a/sysdeps/unix/sysv/linux/rseq-internal.h
>> +++ b/sysdeps/unix/sysv/linux/rseq-internal.h
>> @@ -108,13 +108,12 @@ rseq_register_current_thread (struct pthread *self, bool do_rseq)
>>        if (size < RSEQ_AREA_SIZE_INITIAL)
>>          size = RSEQ_AREA_SIZE_INITIAL;
>>  
>> -      /* Initialize the rseq fields that are read by the kernel on
>> -         registration, there is no guarantee that struct pthread is
>> -         cleared on all architectures.  */
>> +      /* Initialize the whole rseq area to zero prior to registration.  */
>> +      memset (RSEQ_SELF (), 0, size);
>> +
>> +      /* Set the cpu_id field to RSEQ_CPU_ID_UNINITIALIZED, this is checked by
>> +         the kernel at registration when CONFIG_DEBUG_RSEQ is enabled.  */
>>        RSEQ_SETMEM (cpu_id, RSEQ_CPU_ID_UNINITIALIZED);
>> -      RSEQ_SETMEM (cpu_id_start, 0);
>> -      RSEQ_SETMEM (rseq_cs, 0);
>> -      RSEQ_SETMEM (flags, 0);
>>  
>>        int ret = INTERNAL_SYSCALL_CALL (rseq, RSEQ_SELF (), size, 0, RSEQ_SIG);
>>        if (!INTERNAL_SYSCALL_ERROR_P (ret))
> 
> The change looks fine to me in principle, just one observation:
> 
> I think it might be safer to add #include <dl-symbol-redir-ifunc.h>
> to sysdeps/nptl/dl-tls_init_tp.c (where rseq_register_current_thread is
> called on the main thread) because memset might need a more initialzed
> TCB than what we have at this point.

I'll add this and send a v2.

Thanks,

Michael