Re: [PATCH glibc 2.31 1/5] glibc: Perform rseq(2) registration at C startup and thread creation (v12)

[Florian Weimer]

* Mathieu Desnoyers:

> I'm unsure whether there are changes I need to do in my rseq patchset, or
> if this is a separate issue that will be fixed separately before glibc 2.31
> is out, which would then update the rseq bits accordingly ?

Someone else (perhaps me) has to fix __libc_multiple_libcs.  Then you
can use it instead/in addition to the rtld_active check (depending on
the semantics we agree upon for __libc_multiple_libcs).

Fixing __libc_multiple_libcs may also address the early initialization
issue because for that to be always correct, we need to run the
initialization code before ELF constructors.

>>> I'm less convinced that we actually need this.  I don't think we have
>>> ever done anything like that before, and I don't think it's necessary.
>>> Any secondary rseq library just needs to note if it could perform
>>> registration, and if it failed to do so, do not perform unregistration
>>> in a pthread destructor callback.
>
> If that secondary rseq library happens to try to perform registration within
> its library constructor (before glibc has performed the __rseq_abi TLS
> registration), we end up in a situation where the secondary library takes
> ownership of rseq, even though libc would require ownership. This is a
> scenario we want to avoid.

We can avoid that if we run the glibc initialization before user code
(except IFUNC resolvers).  glibc itself doesn't have to do the
initialization from an ELF constructor.

> Making sure libc reserves ownership through __rseq_handled (which is
> a non-TLS variable that can be accessed early in the program lifetime)
> protects against this.

If that's it's only purpose, I don't think it's necessary.  If the
kernel can fail the second registration attempt, that would be all the
information the alternative rseq implementation needs (plus the matter
of destruction).

Thanks,
Florian

Re: [PATCH glibc 2.31 1/5] glibc: Perform rseq(2) registration at C startup and thread creation (v12)

[Mathieu Desnoyers]

- On Sep 11, 2019, at 3:00 PM, carlos car...@redhat.com wrote:

> On 9/11/19 2:26 PM, Florian Weimer wrote:
>> * Mathieu Desnoyers:
>> 
>>> +#ifdef SHARED
>>> +  if (rtld_active ())
>>> +{
>>> +  /* Register rseq ABI to the kernel.   */
>>> +  (void) rseq_register_current_thread ();
>>> +}
>>> +#else
>> 
>> I think this will need *another* check for the inner libc in an audit
>> module.  See what we do in malloc.  __libc_multiple_libcs is supposed to
>> cover that, but it's unfortunately not reliable.
>> 
>> I believe without that additional check, the first audit module we load
>> will claim rseq, and the main program will not have control over the
>> rseq area.  Reversed roles would be desirable here.
>> 
>> In October, I hope to fix __libc_multiple_libcs, and then you can use
>> just that.  (We have a Fedora patch that is supposed to fix it, I need
>> to documen the mechanism and upstream it.)
> 
> This is a technical issue we can resolve.

I'm unsure whether there are changes I need to do in my rseq patchset, or
if this is a separate issue that will be fixed separately before glibc 2.31
is out, which would then update the rseq bits accordingly ?

> 
>>> +/* Advertise Restartable Sequences registration ownership across
>>> +   application and shared libraries.
>>> +
>>> +   Libraries and applications must check whether this variable is zero or
>>> +   non-zero if they wish to perform rseq registration on their own. If it
>>> +   is zero, it means restartable sequence registration is not handled, and
>>> +   the library or application is free to perform rseq registration. In
>>> +   that case, the library or application is taking ownership of rseq
>>> +   registration, and may set __rseq_handled to 1. It may then set it back
>>> +   to 0 after it completes unregistering rseq.
>>> +
>>> +   If __rseq_handled is found to be non-zero, it means that another
>>> +   library (or the application) is currently handling rseq registration.
>>> +
>>> +   Typical use of __rseq_handled is within library constructors and
>>> +   destructors, or at program startup.  */
>>> +
>>> +int __rseq_handled;
>> 
>> Are there any programs that use __rseq_handled *today*?

No, because I told all open source project developers asking whether they
can use rseq to wait until we agree on _this_ precise user-level ABI
(__rseq_abi and __rseq_handled). Until we agree on this, there _can_
be no users, unless they are willing to suffer conflicts when their
application/program is linked against an updated glibc.

>> 
>> I'm less convinced that we actually need this.  I don't think we have
>> ever done anything like that before, and I don't think it's necessary.
>> Any secondary rseq library just needs to note if it could perform
>> registration, and if it failed to do so, do not perform unregistration
>> in a pthread destructor callback.

If that secondary rseq library happens to try to perform registration within
its library constructor (before glibc has performed the __rseq_abi TLS
registration), we end up in a situation where the secondary library takes
ownership of rseq, even though libc would require ownership. This is a
scenario we want to avoid.

Making sure libc reserves ownership through __rseq_handled (which is
a non-TLS variable that can be accessed early in the program lifetime)
protects against this.

>> 
>> Sure, there's the matter of pthread destructor ordering, but that
>> problem is not different from any other singleton (thread-local or not),
>> and the fix for the last time this has come up (TLS destructors vs
>> dlclose) was to upgrade glibc.
> 
> This is a braoder issue.
> 
> Mathieu,
> 
> It would be easier to merge the patch set if it were just an unconditional
> registration like we do for set_robust_list().
> 
> What's your thought there?

I don't expect set_robust_list was really useful without glibc support.
In the current case, rseq can be used by applications and libraries even
with older glibc. My goal is to enable such use and not wait for years
before end-users upgrade their glibc before rseq can be used.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH glibc 2.31 1/5] glibc: Perform rseq(2) registration at C startup and thread creation (v12)

[Rich Felker]

On Wed, Sep 11, 2019 at 09:54:23PM +0200, Florian Weimer wrote:
> * Carlos O'Donell:
> 
> > On 9/11/19 3:08 PM, Florian Weimer wrote:
> >> * Carlos O'Donell:
> >> 
> >>> It would be easier to merge the patch set if it were just an unconditional
> >>> registration like we do for set_robust_list().
> >> 
> >> Note that this depends on the in-tree system call numbers list, which I
> >> still need to finish according to Joseph's specifications.
> >
> > Which work is this? Do you have a URL reference to WIP?
> 
>   
>   
> 
> I think realistically this is needed for the Y2038 work as well if we
> want to support building glibc with older kernel headers.  “glibc 2..31
> will have Y2038 support and rseq support, but only if it runs on a
> current and it happens to have been built against sufficiently recent
> kernel headers” is a bit difficult to explain.  The current kernel part
> is easy enough to understand, but the impact of the kernel headers on
> the feature set has always been tough to explain.  Especially if you
> factor in vendor kernels with system call backports.

I'm in favor of in-tree syscall numbers list. If you don't want O(n)
per-arch work, though, you could just define the 'base number' for
each arch and use the fact that all the new syscalls share a common
numbering (i.e. base + constant depending only on syscall). I think
including the list with glibc is more robust though, and would
eliminate the need to check for definitions of older (pre-unification)
syscalls glibc wants to use.

Rich

Re: [PATCH glibc 2.31 1/5] glibc: Perform rseq(2) registration at C startup and thread creation (v12)

[Florian Weimer]

* Florian Weimer:

> * Carlos O'Donell:
>
>> On 9/11/19 3:08 PM, Florian Weimer wrote:
>>> * Carlos O'Donell:
>>> 
 It would be easier to merge the patch set if it were just an unconditional
 registration like we do for set_robust_list().
>>> 
>>> Note that this depends on the in-tree system call numbers list, which I
>>> still need to finish according to Joseph's specifications.
>>
>> Which work is this? Do you have a URL reference to WIP?
>
>   
>   

Sorry, there was also this:

  

I also posted a build-many-glibcs.py patch at some point with an
automatic table update, but that still had the massive bot-cycle time.

My current line of thinking is to implement some --use-compilers flag,
so that you can build a fresh glibc checkout against an old, pre-built
compilers for the system call tables update, and then use the patched
glibc sources for one (and hopefully final) bot-cycle.

Thanks,
Florian

Re: [PATCH glibc 2.31 1/5] glibc: Perform rseq(2) registration at C startup and thread creation (v12)

[Florian Weimer]

* Carlos O'Donell:

> On 9/11/19 3:08 PM, Florian Weimer wrote:
>> * Carlos O'Donell:
>> 
>>> It would be easier to merge the patch set if it were just an unconditional
>>> registration like we do for set_robust_list().
>> 
>> Note that this depends on the in-tree system call numbers list, which I
>> still need to finish according to Joseph's specifications.
>
> Which work is this? Do you have a URL reference to WIP?

  
  

I think realistically this is needed for the Y2038 work as well if we
want to support building glibc with older kernel headers.  “glibc 2.31
will have Y2038 support and rseq support, but only if it runs on a
current and it happens to have been built against sufficiently recent
kernel headers” is a bit difficult to explain.  The current kernel part
is easy enough to understand, but the impact of the kernel headers on
the feature set has always been tough to explain.  Especially if you
factor in vendor kernels with system call backports.

Thanks,
Florian

Re: [PATCH glibc 2.31 1/5] glibc: Perform rseq(2) registration at C startup and thread creation (v12)

[Carlos O'Donell]

On 9/11/19 3:08 PM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> It would be easier to merge the patch set if it were just an unconditional
>> registration like we do for set_robust_list().
> 
> Note that this depends on the in-tree system call numbers list, which I
> still need to finish according to Joseph's specifications.

Which work is this? Do you have a URL reference to WIP?

> (We have something that should work for us as long as we can get large
> machines from the lab, but I agree that it's not very useful if glibc
> bot-cycle time is roughly one business day.)

Yeah, we have to discuss how to accelerate this.

-- 
Cheers,
Carlos.

Re: [PATCH glibc 2.31 1/5] glibc: Perform rseq(2) registration at C startup and thread creation (v12)

[Florian Weimer]

* Carlos O'Donell:

> It would be easier to merge the patch set if it were just an unconditional
> registration like we do for set_robust_list().

Note that this depends on the in-tree system call numbers list, which I
still need to finish according to Joseph's specifications.

(We have something that should work for us as long as we can get large
machines from the lab, but I agree that it's not very useful if glibc
bot-cycle time is roughly one business day.)

Thanks,
Florian

Re: [PATCH glibc 2.31 1/5] glibc: Perform rseq(2) registration at C startup and thread creation (v12)

[Carlos O'Donell]

On 9/11/19 2:26 PM, Florian Weimer wrote:
> * Mathieu Desnoyers:
> 
>> +#ifdef SHARED
>> +  if (rtld_active ())
>> +{
>> +  /* Register rseq ABI to the kernel.   */
>> +  (void) rseq_register_current_thread ();
>> +}
>> +#else
> 
> I think this will need *another* check for the inner libc in an audit
> module.  See what we do in malloc.  __libc_multiple_libcs is supposed to
> cover that, but it's unfortunately not reliable.
> 
> I believe without that additional check, the first audit module we load
> will claim rseq, and the main program will not have control over the
> rseq area.  Reversed roles would be desirable here.
> 
> In October, I hope to fix __libc_multiple_libcs, and then you can use
> just that.  (We have a Fedora patch that is supposed to fix it, I need
> to documen the mechanism and upstream it.)

This is a technical issue we can resolve.

>> +/* Advertise Restartable Sequences registration ownership across
>> +   application and shared libraries.
>> +
>> +   Libraries and applications must check whether this variable is zero or
>> +   non-zero if they wish to perform rseq registration on their own. If it
>> +   is zero, it means restartable sequence registration is not handled, and
>> +   the library or application is free to perform rseq registration. In
>> +   that case, the library or application is taking ownership of rseq
>> +   registration, and may set __rseq_handled to 1. It may then set it back
>> +   to 0 after it completes unregistering rseq.
>> +
>> +   If __rseq_handled is found to be non-zero, it means that another
>> +   library (or the application) is currently handling rseq registration.
>> +
>> +   Typical use of __rseq_handled is within library constructors and
>> +   destructors, or at program startup.  */
>> +
>> +int __rseq_handled;
> 
> Are there any programs that use __rseq_handled *today*?
> 
> I'm less convinced that we actually need this.  I don't think we have
> ever done anything like that before, and I don't think it's necessary.
> Any secondary rseq library just needs to note if it could perform
> registration, and if it failed to do so, do not perform unregistration
> in a pthread destructor callback.
> 
> Sure, there's the matter of pthread destructor ordering, but that
> problem is not different from any other singleton (thread-local or not),
> and the fix for the last time this has come up (TLS destructors vs
> dlclose) was to upgrade glibc.

This is a braoder issue.

Mathieu,

It would be easier to merge the patch set if it were just an unconditional
registration like we do for set_robust_list().

What's your thought there?

-- 
Cheers,
Carlos.

Re: [PATCH glibc 2.31 1/5] glibc: Perform rseq(2) registration at C startup and thread creation (v12)

[Florian Weimer]

* Mathieu Desnoyers:

> +#ifdef SHARED
> +  if (rtld_active ())
> +{
> +  /* Register rseq ABI to the kernel.   */
> +  (void) rseq_register_current_thread ();
> +}
> +#else

I think this will need *another* check for the inner libc in an audit
module.  See what we do in malloc.  __libc_multiple_libcs is supposed to
cover that, but it's unfortunately not reliable.

I believe without that additional check, the first audit module we load
will claim rseq, and the main program will not have control over the
rseq area.  Reversed roles would be desirable here.

In October, I hope to fix __libc_multiple_libcs, and then you can use
just that.  (We have a Fedora patch that is supposed to fix it, I need
to documen the mechanism and upstream it.)

> +/* Advertise Restartable Sequences registration ownership across
> +   application and shared libraries.
> +
> +   Libraries and applications must check whether this variable is zero or
> +   non-zero if they wish to perform rseq registration on their own. If it
> +   is zero, it means restartable sequence registration is not handled, and
> +   the library or application is free to perform rseq registration. In
> +   that case, the library or application is taking ownership of rseq
> +   registration, and may set __rseq_handled to 1. It may then set it back
> +   to 0 after it completes unregistering rseq.
> +
> +   If __rseq_handled is found to be non-zero, it means that another
> +   library (or the application) is currently handling rseq registration.
> +
> +   Typical use of __rseq_handled is within library constructors and
> +   destructors, or at program startup.  */
> +
> +int __rseq_handled;

Are there any programs that use __rseq_handled *today*?

I'm less convinced that we actually need this.  I don't think we have
ever done anything like that before, and I don't think it's necessary.
Any secondary rseq library just needs to note if it could perform
registration, and if it failed to do so, do not perform unregistration
in a pthread destructor callback.

Sure, there's the matter of pthread destructor ordering, but that
problem is not different from any other singleton (thread-local or not),
and the fix for the last time this has come up (TLS destructors vs
dlclose) was to upgrade glibc.

Thanks,
Florian