| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ee987900-85ff-49f4-b393-4bbb889554dc@linuxfoundation.org>
Date: Thu, 3 Oct 2024 16:40:44 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>
Cc: linux-kernel@...r.kernel.org, "Paul E. McKenney" <paulmck@...nel.org>,
Boqun Feng <boqun.feng@...il.com>, Valentin Schneider <vschneid@...hat.com>,
Mel Gorman <mgorman@...e.de>, Steven Rostedt <rostedt@...dmis.org>,
Vincent Guittot <vincent.guittot@...aro.org>,
Dietmar Eggemann <dietmar.eggemann@....com>, Ben Segall
<bsegall@...gle.com>, Yury Norov <yury.norov@...il.com>,
Rasmus Villemoes <linux@...musvillemoes.dk>,
"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
Carlos O'Donell <carlos@...hat.com>, Florian Weimer <fweimer@...hat.com>,
Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH 2/2] selftests/rseq: Adapt to glibc __rseq_size feature
detection
On 10/3/24 11:51, 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)")
>
> Wihout this fix, rseq selftests for mm_cid fail:
Without
Can you change the short log to say "Fix mm_cid test failure"
This way it is clear that this fixes a test failure. You can
add "Adapt to glibc __rseq_size feature detection: in the
chabeg log for context.
>
> ./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@...icios.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> CC: Boqun Feng <boqun.feng@...il.com>
> CC: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
> Cc: Shuah Khan <skhan@...uxfoundation.org>
> CC: Carlos O'Donell <carlos@...hat.com>
> CC: Florian Weimer <fweimer@...hat.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)
thanks,
-- Shuah
Powered by blists - more mailing lists