[v12,01/17] nptl: Add rseq auxvals
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
Get the rseq feature size and alignment requirement from the auxiliary
vector for use inside the dynamic loader.
This will be used in the TLS block allocator to compute the size and
alignment of the rseq area block for the extended ABI support.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
---
Changes since v11:
- Removed _dl_rseq_feature_size, use __rseq_size instead
- Replace GLRO(dl_rseq_align) with a hidden global variable _rseq_align
---
sysdeps/nptl/dl-tls_init_tp.c | 2 ++
sysdeps/unix/sysv/linux/dl-parse_auxv.h | 5 +++++
sysdeps/unix/sysv/linux/rseq-internal.h | 17 ++++++++++++++---
3 files changed, 21 insertions(+), 3 deletions(-)
Comments
* Michael Jeanson:
> diff --git a/sysdeps/unix/sysv/linux/dl-parse_auxv.h b/sysdeps/unix/sysv/linux/dl-parse_auxv.h
> index ea2a58ecb1..d24daab3fc 100644
> --- a/sysdeps/unix/sysv/linux/dl-parse_auxv.h
> +++ b/sysdeps/unix/sysv/linux/dl-parse_auxv.h
> @@ -21,6 +21,7 @@
> #include <fpu_control.h>
> #include <ldsodefs.h>
> #include <link.h>
> +#include <rseq-internal.h>
>
> typedef ElfW(Addr) dl_parse_auxv_t[AT_MINSIGSTKSZ + 1];
>
> @@ -59,5 +60,9 @@ void _dl_parse_auxv (ElfW(auxv_t) *av, dl_parse_auxv_t auxv_values)
> GLRO(dl_sysinfo) = auxv_values[AT_SYSINFO];
> #endif
>
> + _rseq_size = MAX (auxv_values[AT_RSEQ_FEATURE_SIZE],
> + RSEQ_AREA_SIZE_INITIAL_USED);
> + _rseq_align = MAX (auxv_values[AT_RSEQ_ALIGN], RSEQ_MIN_ALIGN);
> +
> DL_PLATFORM_AUXV
> }
I believe you need an else branch in __tls_init_tp in
sysdeps/nptl/dl-tls_init_tp.c, to set _rseq_size to 0 if registration
fails:
bool do_rseq = true;
do_rseq = TUNABLE_GET (rseq, int, NULL);
if (rseq_register_current_thread (pd, do_rseq))
_rseq_size = RSEQ_AREA_SIZE_INITIAL_USED;
Not sure why this doesn't result in a test failure (maybe it's fixed
again later in the series?).
Thanks,
Florian
On 2024-08-06 02 h 11, Florian Weimer wrote:
> * Michael Jeanson:
>
>> diff --git a/sysdeps/unix/sysv/linux/dl-parse_auxv.h b/sysdeps/unix/sysv/linux/dl-parse_auxv.h
>> index ea2a58ecb1..d24daab3fc 100644
>> --- a/sysdeps/unix/sysv/linux/dl-parse_auxv.h
>> +++ b/sysdeps/unix/sysv/linux/dl-parse_auxv.h
>> @@ -21,6 +21,7 @@
>> #include <fpu_control.h>
>> #include <ldsodefs.h>
>> #include <link.h>
>> +#include <rseq-internal.h>
>>
>> typedef ElfW(Addr) dl_parse_auxv_t[AT_MINSIGSTKSZ + 1];
>>
>> @@ -59,5 +60,9 @@ void _dl_parse_auxv (ElfW(auxv_t) *av, dl_parse_auxv_t auxv_values)
>> GLRO(dl_sysinfo) = auxv_values[AT_SYSINFO];
>> #endif
>>
>> + _rseq_size = MAX (auxv_values[AT_RSEQ_FEATURE_SIZE],
>> + RSEQ_AREA_SIZE_INITIAL_USED);
>> + _rseq_align = MAX (auxv_values[AT_RSEQ_ALIGN], RSEQ_MIN_ALIGN);
>> +
>> DL_PLATFORM_AUXV
>> }
>
> I believe you need an else branch in __tls_init_tp in
> sysdeps/nptl/dl-tls_init_tp.c, to set _rseq_size to 0 if registration
> fails:
>
> bool do_rseq = true;
> do_rseq = TUNABLE_GET (rseq, int, NULL);
> if (rseq_register_current_thread (pd, do_rseq))
> _rseq_size = RSEQ_AREA_SIZE_INITIAL_USED;
>
> Not sure why this doesn't result in a test failure (maybe it's fixed
> again later in the series?).
Before the removal of _dl_rseq_feature_size there was no behavior change
introduced until "Move the rseq area to the 'extra TLS' block" which is
where _rseq_size is set to 0 on failure. I agree it would make more
sense to have it in this patch, I'll queue this change for the next round.
Thanks,
Michael
On 2024-08-06 09:32, Michael Jeanson wrote:
> Before the removal of _dl_rseq_feature_size there was no behavior change
> introduced until "Move the rseq area to the 'extra TLS' block" which is where
> _rseq_size is set to 0 on failure. I agree it would make more sense to have it
> in this patch, I'll queue this change for the next round.
I was wondering if this patch with the code to set _rseq_size to zero on
registration failure, plus an additional modification to cap the _rseq_size to
32 (the size of the rseq area in the struct pthread) could be backported to
2.40 and earlier? It would give application code access to the 'node_id' and
'mm_cid' fields without introducing the whole 'extra tls' stuff.
Thoughts?
Thanks,
Michael
* Michael Jeanson:
> On 2024-08-06 09:32, Michael Jeanson wrote:
>> Before the removal of _dl_rseq_feature_size there was no behavior
>> change introduced until "Move the rseq area to the 'extra TLS'
>> block" which is where _rseq_size is set to 0 on failure. I agree it
>> would make more sense to have it in this patch, I'll queue this
>> change for the next round.
>
> I was wondering if this patch with the code to set _rseq_size to zero
> on registration failure, plus an additional modification to cap the
> _rseq_size to 32 (the size of the rseq area in the struct pthread)
> could be backported to 2.40 and earlier? It would give application
> code access to the 'node_id' and 'mm_cid' fields without introducing
> the whole 'extra tls' stuff.
We already touched briefly on this. I was worried that kernels would
only support two size arguments in the rseq system call: 32 and what
they signal through auxiliary vector. And that passing 32 might
actually mean 20 or 28 in a future kernel release, not 32. Then __rseq
would be wrong again.
Thanks,
Florian
On 2024-08-07 13:52, Florian Weimer wrote:
> We already touched briefly on this. I was worried that kernels would
> only support two size arguments in the rseq system call: 32 and what
> they signal through auxiliary vector. And that passing 32 might
> actually mean 20 or 28 in a future kernel release, not 32. Then __rseq
> would be wrong again.
According to Mathieu, the plan for hypothetical future kernels with a feature
size above 32 is to have 2 types of registration:
- fixed 32 bytes, gives you access to ALL the fields that fit inside 32 bytes
- at least the feature size reported in the auxiliary vector
In the first case, reporting a feature size of 32 would be correct but I can
see an issue with applications special casing '__rseq_size == 32' since it's
what glibc < 2.40 reports and it's not a valid 'feature size' when reported by
those libc's.
Also the plan kernel side is to never have a feature size of 32 to allow
userspace to special case it, but until such a kernel ships some other
consideration could override this I suppose. We'll have to handle this when we
get there.
What we could do here is set a MAX of 28 instead of 32 on '__rseq_size' in
this patch which would still give us access to all the fields that are
currently available in released kernels without the confusion associated with
'32'.
Thanks,
Michael
* Michael Jeanson:
> On 2024-08-06 09:32, Michael Jeanson wrote:
>> Before the removal of _dl_rseq_feature_size there was no behavior
>> change introduced until "Move the rseq area to the 'extra TLS'
>> block" which is where _rseq_size is set to 0 on failure. I agree it
>> would make more sense to have it in this patch, I'll queue this
>> change for the next round.
>
> I was wondering if this patch with the code to set _rseq_size to zero
> on registration failure, plus an additional modification to cap the
> _rseq_size to 32 (the size of the rseq area in the struct pthread)
> could be backported to 2.40 and earlier? It would give application
> code access to the 'node_id' and 'mm_cid' fields without introducing
> the whole 'extra tls' stuff.
>
> Thoughts?
I thought about this some more. I think we should also increase the
reservation inside struct pthread beyond 32 bytes if we do this, so that
we avoid the __rseq_size == 32 ambiguity. It currently doesn't break
the GLIBC_PRIVATE ABI because rseq_area is at the end of struct pthread.
(As long as we assume that ld.so gets updated before libc.so because it
comes first in lexicographic order.)
Thanks,
Florian
On 2024-08-15 13:54, Florian Weimer wrote:
> * Michael Jeanson:
>
>> On 2024-08-06 09:32, Michael Jeanson wrote:
>>> Before the removal of _dl_rseq_feature_size there was no behavior
>>> change introduced until "Move the rseq area to the 'extra TLS'
>>> block" which is where _rseq_size is set to 0 on failure. I agree it
>>> would make more sense to have it in this patch, I'll queue this
>>> change for the next round.
>>
>> I was wondering if this patch with the code to set _rseq_size to zero
>> on registration failure, plus an additional modification to cap the
>> _rseq_size to 32 (the size of the rseq area in the struct pthread)
>> could be backported to 2.40 and earlier? It would give application
>> code access to the 'node_id' and 'mm_cid' fields without introducing
>> the whole 'extra tls' stuff.
>>
>> Thoughts?
>
> I thought about this some more. I think we should also increase the
> reservation inside struct pthread beyond 32 bytes if we do this, so that
> we avoid the __rseq_size == 32 ambiguity. It currently doesn't break
> the GLIBC_PRIVATE ABI because rseq_area is at the end of struct pthread.
> (As long as we assume that ld.so gets updated before libc.so because it
> comes first in lexicographic order.)
So the plan for 2.40 would look something like this?
- Increase the size of the rseq area in 'struct pthread' to 64 bytes or more?
- Set __rseq_size from AT_RSEQ_FEATURE_SIZE with a min of 20 and max of 64
The resulting __rseq_size for different kernels:
- kernel < 6.3 (AT_RSEQ_FEATURE_SIZE == 0) : __rseq_size == 20
- Same behavior as current 2.40
- kernel >= 6.3 (AT_RSEQ_FEATURE_SIZE == 28) : __rseq_size == 28
- Exposes 'node_id' and 'mm_cid' to userspace
- future unreleased kernel (AT_RSEQ_FEATURE_SIZE == 32) : __rseq_size == 32
- We expect such a kernel to never exist, but would technically work
- future unreleased kernel (AT_RSEQ_FEATURE_SIZE == 36) : __rseq_size == 36
- Exposes future fields to userspace
- future unreleased kernel (AT_RSEQ_FEATURE_SIZE > 64) : __rseq_size == 0
- Attempting registration with a kernel that has a feature size bigger than
our rseq area would result in a failed registration. We don't deal with
the planned compat mode of 32 bytes.
Does that align with what you have in mind?
Thanks,
Michael
@@ -46,6 +46,8 @@ rtld_mutex_dummy (pthread_mutex_t *lock)
const unsigned int __rseq_flags;
+size_t _rseq_align attribute_hidden;
+
void
__tls_pre_init_tp (void)
{
@@ -21,6 +21,7 @@
#include <fpu_control.h>
#include <ldsodefs.h>
#include <link.h>
+#include <rseq-internal.h>
typedef ElfW(Addr) dl_parse_auxv_t[AT_MINSIGSTKSZ + 1];
@@ -59,5 +60,9 @@ void _dl_parse_auxv (ElfW(auxv_t) *av, dl_parse_auxv_t auxv_values)
GLRO(dl_sysinfo) = auxv_values[AT_SYSINFO];
#endif
+ _rseq_size = MAX (auxv_values[AT_RSEQ_FEATURE_SIZE],
+ RSEQ_AREA_SIZE_INITIAL_USED);
+ _rseq_align = MAX (auxv_values[AT_RSEQ_ALIGN], RSEQ_MIN_ALIGN);
+
DL_PLATFORM_AUXV
}
@@ -25,12 +25,23 @@
#include <stdio.h>
#include <sys/rseq.h>
-/* 32 is the initially required value for the area size. The
- actually used rseq size may be less (20 bytes initially). */
+/* Minimum size of the rseq area allocation required by the syscall. The
+ actually used rseq feature size may be less (20 bytes initially). */
#define RSEQ_AREA_SIZE_INITIAL 32
+
+/* Minimum used feature size of the rseq area. */
#define RSEQ_AREA_SIZE_INITIAL_USED 20
-/* The variables are in .data.relro but are not yet write-protected. */
+/* Minimum alignment of the rseq area. */
+#define RSEQ_MIN_ALIGN 32
+
+/* Alignment requirement of the rseq area.
+ Populated from the auxiliary vector with a minimum of '32'. */
+extern size_t _rseq_align attribute_hidden;
+
+/* Size of the active features in the rseq area.
+ Populated from the auxiliary vector with a minimum of '20'.
+ In .data.relro but not yet write-protected. */
extern unsigned int _rseq_size attribute_hidden;
extern ptrdiff_t _rseq_offset attribute_hidden;