Adapt the rseq.c/rseq.h code to follow GNU C library changes introduced by:
commit 2e456ccf0c34 ("Linux: Make __rseq_size useful for feature detection (bug 31965)")
Without this fix, rseq selftests for mm_cid fail:
./run_param_test.sh
Default parameters
Running test spinlock
Running compare-twice test spinlock
Running mm_cid test spinlock
Error: cpu id getter unavailable
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Peter Zijlstra <peterz@infradead.org>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>
CC: Carlos O'Donell <carlos@redhat.com>
CC: Florian Weimer <fweimer@redhat.com>
---
tools/testing/selftests/rseq/rseq.c | 109 +++++++++++++++++++---------
tools/testing/selftests/rseq/rseq.h | 10 +--
2 files changed, 76 insertions(+), 43 deletions(-)
diff --git a/tools/testing/selftests/rseq/rseq.c b/tools/testing/selftests/rseq/rseq.c
index 96e812bdf8a4..3797bb0881da 100644
--- a/tools/testing/selftests/rseq/rseq.c
+++ b/tools/testing/selftests/rseq/rseq.c
@@ -60,12 +60,6 @@ unsigned int rseq_size = -1U;
/* Flags used during rseq registration. */
unsigned int rseq_flags;
-/*
- * rseq feature size supported by the kernel. 0 if the registration was
- * unsuccessful.
- */
-unsigned int rseq_feature_size = -1U;
-
static int rseq_ownership;
static int rseq_reg_success; /* At least one rseq registration has succeded. */
@@ -111,6 +105,43 @@ int rseq_available(void)
}
}
+/* The rseq areas need to be at least 32 bytes. */
+static
+unsigned get_rseq_min_alloc_size(void)
+{
+ unsigned int alloc_size = rseq_size;
+
+ if (alloc_size < ORIG_RSEQ_ALLOC_SIZE)
+ alloc_size = ORIG_RSEQ_ALLOC_SIZE;
+ return alloc_size;
+}
+
+/*
+ * Return the feature size supported by the kernel.
+ *
+ * Depending on the value returned by getauxval(AT_RSEQ_FEATURE_SIZE):
+ *
+ * 0: Return ORIG_RSEQ_FEATURE_SIZE (20)
+ * > 0: Return the value from getauxval(AT_RSEQ_FEATURE_SIZE).
+ *
+ * It should never return a value below ORIG_RSEQ_FEATURE_SIZE.
+ */
+static
+unsigned int get_rseq_kernel_feature_size(void)
+{
+ unsigned long auxv_rseq_feature_size, auxv_rseq_align;
+
+ auxv_rseq_align = getauxval(AT_RSEQ_ALIGN);
+ assert(!auxv_rseq_align || auxv_rseq_align <= RSEQ_THREAD_AREA_ALLOC_SIZE);
+
+ auxv_rseq_feature_size = getauxval(AT_RSEQ_FEATURE_SIZE);
+ assert(!auxv_rseq_feature_size || auxv_rseq_feature_size <= RSEQ_THREAD_AREA_ALLOC_SIZE);
+ if (auxv_rseq_feature_size)
+ return auxv_rseq_feature_size;
+ else
+ return ORIG_RSEQ_FEATURE_SIZE;
+}
+
int rseq_register_current_thread(void)
{
int rc;
@@ -119,7 +150,7 @@ int rseq_register_current_thread(void)
/* Treat libc's ownership as a successful registration. */
return 0;
}
- rc = sys_rseq(&__rseq_abi, rseq_size, 0, RSEQ_SIG);
+ rc = sys_rseq(&__rseq_abi, get_rseq_min_alloc_size(), 0, RSEQ_SIG);
if (rc) {
if (RSEQ_READ_ONCE(rseq_reg_success)) {
/* Incoherent success/failure within process. */
@@ -140,28 +171,12 @@ int rseq_unregister_current_thread(void)
/* Treat libc's ownership as a successful unregistration. */
return 0;
}
- rc = sys_rseq(&__rseq_abi, rseq_size, RSEQ_ABI_FLAG_UNREGISTER, RSEQ_SIG);
+ rc = sys_rseq(&__rseq_abi, get_rseq_min_alloc_size(), RSEQ_ABI_FLAG_UNREGISTER, RSEQ_SIG);
if (rc)
return -1;
return 0;
}
-static
-unsigned int get_rseq_feature_size(void)
-{
- unsigned long auxv_rseq_feature_size, auxv_rseq_align;
-
- auxv_rseq_align = getauxval(AT_RSEQ_ALIGN);
- assert(!auxv_rseq_align || auxv_rseq_align <= RSEQ_THREAD_AREA_ALLOC_SIZE);
-
- auxv_rseq_feature_size = getauxval(AT_RSEQ_FEATURE_SIZE);
- assert(!auxv_rseq_feature_size || auxv_rseq_feature_size <= RSEQ_THREAD_AREA_ALLOC_SIZE);
- if (auxv_rseq_feature_size)
- return auxv_rseq_feature_size;
- else
- return ORIG_RSEQ_FEATURE_SIZE;
-}
-
static __attribute__((constructor))
void rseq_init(void)
{
@@ -178,28 +193,53 @@ void rseq_init(void)
}
if (libc_rseq_size_p && libc_rseq_offset_p && libc_rseq_flags_p &&
*libc_rseq_size_p != 0) {
+ unsigned int libc_rseq_size;
+
/* rseq registration owned by glibc */
rseq_offset = *libc_rseq_offset_p;
- rseq_size = *libc_rseq_size_p;
+ libc_rseq_size = *libc_rseq_size_p;
rseq_flags = *libc_rseq_flags_p;
- rseq_feature_size = get_rseq_feature_size();
- if (rseq_feature_size > rseq_size)
- rseq_feature_size = rseq_size;
+
+ /*
+ * Previous versions of glibc expose the value
+ * 32 even though the kernel only supported 20
+ * bytes initially. Therefore treat 32 as a
+ * special-case. glibc 2.40 exposes a 20 bytes
+ * __rseq_size without using getauxval(3) to
+ * query the supported size, while still allocating a 32
+ * bytes area. Also treat 20 as a special-case.
+ *
+ * Special-cases are handled by using the following
+ * value as active feature set size:
+ *
+ * rseq_size = min(32, get_rseq_kernel_feature_size())
+ */
+ switch (libc_rseq_size) {
+ case ORIG_RSEQ_FEATURE_SIZE: /* Fallthrough. */
+ case ORIG_RSEQ_ALLOC_SIZE:
+ {
+ unsigned int rseq_kernel_feature_size = get_rseq_kernel_feature_size();
+
+ if (rseq_kernel_feature_size < ORIG_RSEQ_ALLOC_SIZE)
+ rseq_size = rseq_kernel_feature_size;
+ else
+ rseq_size = ORIG_RSEQ_ALLOC_SIZE;
+ break;
+ }
+ default:
+ /* Otherwise just use the __rseq_size from libc as rseq_size. */
+ rseq_size = libc_rseq_size;
+ break;
+ }
return;
}
rseq_ownership = 1;
if (!rseq_available()) {
rseq_size = 0;
- rseq_feature_size = 0;
return;
}
rseq_offset = (void *)&__rseq_abi - rseq_thread_pointer();
rseq_flags = 0;
- rseq_feature_size = get_rseq_feature_size();
- if (rseq_feature_size == ORIG_RSEQ_FEATURE_SIZE)
- rseq_size = ORIG_RSEQ_ALLOC_SIZE;
- else
- rseq_size = RSEQ_THREAD_AREA_ALLOC_SIZE;
}
static __attribute__((destructor))
@@ -209,7 +249,6 @@ void rseq_exit(void)
return;
rseq_offset = 0;
rseq_size = -1U;
- rseq_feature_size = -1U;
rseq_ownership = 0;
}
diff --git a/tools/testing/selftests/rseq/rseq.h b/tools/testing/selftests/rseq/rseq.h
index d7364ea4d201..4e217b620e0c 100644
--- a/tools/testing/selftests/rseq/rseq.h
+++ b/tools/testing/selftests/rseq/rseq.h
@@ -68,12 +68,6 @@ extern unsigned int rseq_size;
/* Flags used during rseq registration. */
extern unsigned int rseq_flags;
-/*
- * rseq feature size supported by the kernel. 0 if the registration was
- * unsuccessful.
- */
-extern unsigned int rseq_feature_size;
-
enum rseq_mo {
RSEQ_MO_RELAXED = 0,
RSEQ_MO_CONSUME = 1, /* Unused */
@@ -193,7 +187,7 @@ static inline uint32_t rseq_current_cpu(void)
static inline bool rseq_node_id_available(void)
{
- return (int) rseq_feature_size >= rseq_offsetofend(struct rseq_abi, node_id);
+ return (int) rseq_size >= rseq_offsetofend(struct rseq_abi, node_id);
}
/*
@@ -207,7 +201,7 @@ static inline uint32_t rseq_current_node_id(void)
static inline bool rseq_mm_cid_available(void)
{
- return (int) rseq_feature_size >= rseq_offsetofend(struct rseq_abi, mm_cid);
+ return (int) rseq_size >= rseq_offsetofend(struct rseq_abi, mm_cid);
}
static inline uint32_t rseq_current_mm_cid(void)
--
2.39.2On 10/3/24 18:44, Mathieu Desnoyers wrote:
> Adapt the rseq.c/rseq.h code to follow GNU C library changes introduced by:
>
> commit 2e456ccf0c34 ("Linux: Make __rseq_size useful for feature detection (bug 31965)")
>
> Without this fix, rseq selftests for mm_cid fail:
>
> ./run_param_test.sh
> Default parameters
> Running test spinlock
> Running compare-twice test spinlock
> Running mm_cid test spinlock
> Error: cpu id getter unavailable
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> CC: Boqun Feng <boqun.feng@gmail.com>
> CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> CC: Carlos O'Donell <carlos@redhat.com>
> CC: Florian Weimer <fweimer@redhat.com>
> ---
> tools/testing/selftests/rseq/rseq.c | 109 +++++++++++++++++++---------
> tools/testing/selftests/rseq/rseq.h | 10 +--
> 2 files changed, 76 insertions(+), 43 deletions(-)
>
Looks good to me.
Acked-by: Shuah Khan <skhan@linuxfoundation.org>
Peter, Ingo - let me know if you plan to take this through
one of your trees. Otherwise I will pick it up.
thanks,
-- Shuah
On 2024-10-04 21:18, Shuah Khan wrote:
> On 10/3/24 18:44, Mathieu Desnoyers wrote:
>> Adapt the rseq.c/rseq.h code to follow GNU C library changes
>> introduced by:
>>
>> commit 2e456ccf0c34 ("Linux: Make __rseq_size useful for feature
>> detection (bug 31965)")
>>
>> Without this fix, rseq selftests for mm_cid fail:
>>
>> ./run_param_test.sh
>> Default parameters
>> Running test spinlock
>> Running compare-twice test spinlock
>> Running mm_cid test spinlock
>> Error: cpu id getter unavailable
>>
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> CC: Boqun Feng <boqun.feng@gmail.com>
>> CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>> Cc: Shuah Khan <skhan@linuxfoundation.org>
>> CC: Carlos O'Donell <carlos@redhat.com>
>> CC: Florian Weimer <fweimer@redhat.com>
>> ---
>> tools/testing/selftests/rseq/rseq.c | 109 +++++++++++++++++++---------
>> tools/testing/selftests/rseq/rseq.h | 10 +--
>> 2 files changed, 76 insertions(+), 43 deletions(-)
>>
>
> Looks good to me.
>
> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
>
> Peter, Ingo - let me know if you plan to take this through
> one of your trees. Otherwise I will pick it up.
Hi Shuah,
I just discussed with Peter on IRC, and if you can pick up
this rseq selftest fix through your tree it would be very much
appreciated,
Thanks,
Mathieu
>
> thanks,
> -- Shuah
>
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
On 10/8/24 08:56, Mathieu Desnoyers wrote:
> On 2024-10-04 21:18, Shuah Khan wrote:
>> On 10/3/24 18:44, Mathieu Desnoyers wrote:
>>> Adapt the rseq.c/rseq.h code to follow GNU C library changes introduced by:
>>>
>>> commit 2e456ccf0c34 ("Linux: Make __rseq_size useful for feature detection (bug 31965)")
>>>
>>> Without this fix, rseq selftests for mm_cid fail:
>>>
>>> ./run_param_test.sh
>>> Default parameters
>>> Running test spinlock
>>> Running compare-twice test spinlock
>>> Running mm_cid test spinlock
>>> Error: cpu id getter unavailable
>>>
>>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> CC: Boqun Feng <boqun.feng@gmail.com>
>>> CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>>> Cc: Shuah Khan <skhan@linuxfoundation.org>
>>> CC: Carlos O'Donell <carlos@redhat.com>
>>> CC: Florian Weimer <fweimer@redhat.com>
>>> ---
>>> tools/testing/selftests/rseq/rseq.c | 109 +++++++++++++++++++---------
>>> tools/testing/selftests/rseq/rseq.h | 10 +--
>>> 2 files changed, 76 insertions(+), 43 deletions(-)
>>>
>>
>> Looks good to me.
>>
>> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
>>
>> Peter, Ingo - let me know if you plan to take this through
>> one of your trees. Otherwise I will pick it up.
>
> Hi Shuah,
>
> I just discussed with Peter on IRC, and if you can pick up
> this rseq selftest fix through your tree it would be very much
> appreciated,
>
Thank you for checking. Looks like it doesn't apply to my tree.
2e456ccf0c34 isn't in 6.12 yet?
Also a couple things you could fix. Please cc linux-kselftest
when you send the next revision with these fixed.
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#114: FILE: tools/testing/selftests/rseq/rseq.c:110:
+unsigned get_rseq_min_alloc_size(void)
WARNING: Prefer 'fallthrough;' over fallthrough comment
#221: FILE: tools/testing/selftests/rseq/rseq.c:218:
+ case ORIG_RSEQ_FEATURE_SIZE: /* Fallthrough. */
thanks,
-- Shuah
On 2024-10-08 23:29, Shuah Khan wrote:
> On 10/8/24 08:56, Mathieu Desnoyers wrote:
>> On 2024-10-04 21:18, Shuah Khan wrote:
>>> On 10/3/24 18:44, Mathieu Desnoyers wrote:
>>>> Adapt the rseq.c/rseq.h code to follow GNU C library changes
>>>> introduced by:
>>>>
>>>> commit 2e456ccf0c34 ("Linux: Make __rseq_size useful for feature
>>>> detection (bug 31965)")
>>>>
>>>> Without this fix, rseq selftests for mm_cid fail:
>>>>
>>>> ./run_param_test.sh
>>>> Default parameters
>>>> Running test spinlock
>>>> Running compare-twice test spinlock
>>>> Running mm_cid test spinlock
>>>> Error: cpu id getter unavailable
>>>>
>>>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>>> CC: Boqun Feng <boqun.feng@gmail.com>
>>>> CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>>>> Cc: Shuah Khan <skhan@linuxfoundation.org>
>>>> CC: Carlos O'Donell <carlos@redhat.com>
>>>> CC: Florian Weimer <fweimer@redhat.com>
>>>> ---
>>>> tools/testing/selftests/rseq/rseq.c | 109
>>>> +++++++++++++++++++---------
>>>> tools/testing/selftests/rseq/rseq.h | 10 +--
>>>> 2 files changed, 76 insertions(+), 43 deletions(-)
>>>>
>>>
>>> Looks good to me.
>>>
>>> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
>>>
>>> Peter, Ingo - let me know if you plan to take this through
>>> one of your trees. Otherwise I will pick it up.
>>
>> Hi Shuah,
>>
>> I just discussed with Peter on IRC, and if you can pick up
>> this rseq selftest fix through your tree it would be very much
>> appreciated,
>>
>
> Thank you for checking. Looks like it doesn't apply to my tree.
>
> 2e456ccf0c34 isn't in 6.12 yet?
That commit is in the glibc project. I will clarify this in my
commit message.
I'll rebase my patch on:
https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git
branch: fixes
>
> Also a couple things you could fix. Please cc linux-kselftest
> when you send the next revision with these fixed.
OK
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #114: FILE: tools/testing/selftests/rseq/rseq.c:110:
> +unsigned get_rseq_min_alloc_size(void)
>
> WARNING: Prefer 'fallthrough;' over fallthrough comment
> #221: FILE: tools/testing/selftests/rseq/rseq.c:218:
> + case ORIG_RSEQ_FEATURE_SIZE: /* Fallthrough. */
>
OK
Thanks,
Mathieu
> thanks,
> -- Shuah
>
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
© 2016 - 2024 Red Hat, Inc.