Re: [PATCH 4/8] nptl: Add rseq registration

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 12/8/21 23:38, Florian Weimer wrote:
> * Siddhesh Poyarekar:
> 
>>> +#ifdef RSEQ_SIG
>>> +static inline void
>>> +rseq_register_current_thread (struct pthread *self)
>>> +{
>>> +  int ret = INTERNAL_SYSCALL_CALL (rseq,
>>> +                                   &self->rseq_area, sizeof (self->rseq_area),
>>> +                                   0, RSEQ_SIG);
>>> +  if (INTERNAL_SYSCALL_ERROR_P (ret))
>>> +    THREAD_SETMEM (self, rseq_area.cpu_id, RSEQ_CPU_ID_REGISTRATION_FAILED);
>>
>> Why can't we just leave it as the kernel did when it failed the
>> syscall?
> 
> The kernel definitely won't write anything if the failure is ENOSYS.  I
> don't expect the kernel to write something for the other failures,
> either.

OK, I interpreted the from the outdated manpage patch[1] that the kernel 
ensures that uninitialized cpu_id will be read as -1.  I read the rseq 
implementation in the kernel and saw that there are a number of error 
paths where the kernel simply returns without touching the user memory. 
  I suppose what they meant by "uninitialized" in the manpage is 
actually "reset after unregister", which is odd.

In any case, what I meant to eventually get at (sorry I wasn't specific; 
I wrote both patch reviews together and didn't realize they'd be read as 
separate emails!) is that RSEQ_CPU_ID_UNINITIALIZED seemed enough for 
all use cases and RSEQ_CPU_ID_REGISTRATION_FAILED seemed unnecessary.

On syscall failure (or tunable being disabled) too it seems safe to do 
THREAD_SETMEM (self, rseq_area.cpu_id, RSEQ_CPU_ID_UNINITIALIZED); 
AFAICT, __tls_init_tp will run early enough that it won't have 
overwritten any earlier rseq calls from user code.

Is there a use case I'm missing?

Thanks,
Siddhesh

[1] https://lkml.org/lkml/2019/2/28/183