Re: [RFC PATCH v4 1/5] glibc: Perform rseq(2) registration at nptl init and thread creation

[Szabolcs Nagy]

On 23/11/18 21:09, Mathieu Desnoyers wrote:
> ----- On Nov 23, 2018, at 1:35 PM, Rich Felker dalias@xxxxxxxx wrote:
> 
>> On Fri, Nov 23, 2018 at 12:52:21PM -0500, Mathieu Desnoyers wrote:
>>> ----- On Nov 23, 2018, at 12:30 PM, Rich Felker dalias@xxxxxxxx wrote:
>>>
>>>> On Fri, Nov 23, 2018 at 12:05:20PM -0500, Mathieu Desnoyers wrote:
>>>>> ----- On Nov 23, 2018, at 9:28 AM, Rich Felker dalias@xxxxxxxx wrote:
>>>>> [...]
>>>>>>
>>>>>> Absolutely. As long as it's in libc, implicit destruction will happen.
>>>>>> Actually I think the glibc code shound unconditionally unregister the
>>>>>> rseq address at exit (after blocking signals, so no application code
>>>>>> can run) in case a third-party rseq library was linked and failed to
>>>>>> do so before thread exit (e.g. due to mismatched ref counts) rather
>>>>>> than respecting the reference count, since it knows it's the last
>>>>>> user. This would make potentially-buggy code safer.
>>>>>
>>>>> OK, let me go ahead with a few ideas/questions along that path.
>>>>                                                 ^^^^^^^^^^^^^^^
>>>>>
>>>>> Let's say our stated goal is to let the "exit" system call from the
>>>>> glibc thread exit path perform rseq unregistration (without explicit
>>>>> unregistration beforehand). Let's look at what we need.
>>>>
>>>> This is not "along that path". The above-quoted text is not about
>>>> assuming it's safe to make SYS_exit without unregistering the rseq
>>>> object, but rather about glibc being able to perform the
>>>> rseq-unregister syscall without caring about reference counts, since
>>>> it knows no other code that might depend on rseq can run after it.
>>>
>>> When saying "along that path", what I mean is: if we go in that direction,
>>> then we should look into going all the way there, and rely on thread
>>> exit to implicitly unregister the TLS area.
>>>
>>> Do you see any reason for doing an explicit unregistration at thread
>>> exit rather than simply rely on the exit system call ?
>>
>> Whether this is needed is an implementation detail of glibc that
>> should be permitted to vary between versions. Unless glibc wants to
>> promise that it would become a public guarantee, it's not part of the
>> discussion around the API/ABI. Only part of the discussion around
>> implementation internals of the glibc rseq stuff.
>>
>> Of course I may be biased thinking application code should not assume
>> this since it's not true on musl -- for detached threads, the thread
>> frees its own stack before exiting (and thus has to unregister
>> set_tid_address and set_robustlist before exiting).
> 
> OK, so on glibc, the implementation could rely on exit side-effect to
> implicitly unregister rseq. On musl, based on the scenario you describe,
> the library should unregister rseq explicitly before stack reclaim.
> 
> Am I understanding the situation correctly ?

i think the point is that you don't need to know these
details in order to come up with a design that allows
both implementations. (then the libc can change later)

so
- is there a need for public unregister api (does the
user do it or the rseq library implicitly unregisters)?
- is there a need for ref counting (or the rseq lib
unconditionally unregisters at the end of a thread,
the libc can certainly do this)?

>>> OK, AFAIU so you argue for leaving the __rseq_abi symbol "weak". Just making
>>> sure I correctly understand your position.
>>
>> I don't think it matters, and I don't think making it weak is
>> meaningful or useful (weak in a shared library is largely meaningless)
>> but maybe I'm missing something here.
> 
> Using a "weak" symbol in early adopter libraries is important, so they
> can be loaded together into the same process without causing loader
> errors due to many definitions of the same strong symbol.
> 
> Using "weak" in a C library is something I'm not sure is a characteristic
> we want or need, because I doubt we would ever want to load two libc at the
> same time in a given process.
> 
> The only reason I see for using "weak" for the __rseq_abi symbol in the
> libc is if we want to allow early adopter applications to define
> __rseq_abi as a strong symbol, which would make some sense.

weak really does not matter in dynamic linking
(unless you set the LD_DYNAMIC_WEAK env var for
backward compat with very old glibc, or if it's
an undefined weak reference)

>> Just blocking at start/exit won't solve the problem because
>> global-dynamic TLS in glibc involves dynamic allocation, which is hard
>> to make AS-safe and of course can fail, leaving no way to make forward
>> progress.
> 
> How hard would it be to create a async-signal-safe memory pool, which would
> be always accessed with signals blocked, so we could fix those corner-cases
> for good ?

that is hard.

in musl tls access is as-safe, but it uses a different
approach: it does all allocations at thread creation or
dlopen time.

glibc has further issues because it supports dlclose
with module unloading and then dynamic tls related
internal structures are hard to free (it is valid to
implement dlclose as a noop, which is what musl does.
tls access needs to synchronize with dlopen and dlclose
when accessing internal structures, but you need a
lock-free mechanism if the access has to be as-safe,
and dlclose is harder to do that way than dlopen)