On 2023-01-05 11:19, Florian Weimer wrote:
* Mathieu Desnoyers:
2- Now about applications (and libc) use of rseq fields:
Using both __rseq_size (from libc) and the result of
getauxval(AT_RSEQ_FEATURE_SIZE), a rseq user can figure which rseq
fields can indeed be used. The important part is how
get_rseq_feature_size() is called in the rseq selftests:
rseq_feature_size = get_rseq_feature_size();
if (rseq_feature_size > rseq_size)
rseq_feature_size = rseq_size;
which basically sets rseq_feature_size to the feature size exposed
by the kernel, except if libc's __rseq_size is smaller than the
feature size exposed by the kernel, in which case it will truncate
the rseq_feature_size to __rseq_size.
Ahh, this happens to work because we pass 32 today from glibc, and
there is nothing left to do in glibc to enable these new fields.
If true, that really argues in favor of this approach.
Yes, you are correct..
Maybe we should just skip the existing padding and use it only for
some vaguely kernel-internal purpose (say through a vDSO helper), so
that it is less of an issue how to communicate the presence of these
fields to userspace.
The fact that libc's __rseq_size is included the original struct
rseq padding is unfortunate, but I really see this as a purely
userspace ABI concern, which should not dictate the layout of the
kernel ABI exposed to user-space, especially given that all the
information required to allow rseq users to know which fields can be
used is readily available by combining the value loaded from
__rseq_size and the result of getauxval(AT_RSEQ_FEATURE_SIZE).
But we must pass size 32 to the kernel today, otherwise rseq
registration fails. It's a kernel-mandated value, not something
that's purely a userspace concern.
What I mean when stating the "userspace concern" is the semantic of the libc's
__rseq_size symbol: whether it means allocated space (including padding)
or actively populated feature fields.
In terms of rseq registration rseq_len argument, here is the updated check
in the system call:
/*
* If there was no rseq previously registered, ensure the provided rseq
* is properly aligned, as communcated to user-space through the ELF
* auxiliary vector AT_RSEQ_ALIGN. If rseq_len is the original rseq
* size, the required alignment is the original struct rseq alignment.
*
* In order to be valid, rseq_len is either the original rseq size, or
* large enough to contain all supported fields, as communicated to
* user-space through the ELF auxiliary vector AT_RSEQ_FEATURE_SIZE.
*/
if (rseq_len < ORIG_RSEQ_SIZE ||
(rseq_len == ORIG_RSEQ_SIZE && !IS_ALIGNED((unsigned long)rseq, ORIG_RSEQ_SIZE)) ||
(rseq_len != ORIG_RSEQ_SIZE && (!IS_ALIGNED((unsigned long)rseq, __alignof__(*rseq)) ||
rseq_len < offsetof(struct rseq, end))))
return -EINVAL;
Which keeps accepting rseq_len=32 (original ABI), else requires that enough space
is available to hold all supported feature fields (but never less than 32 bytes).
Do my explanations take care of your concerns, or are there still aspects that
you are uneasy with ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
