[PATCH v5 09/23] aarch64: Try to free the GCS of makecontext
Carlos O'Donell
codonell@redhat.com
Wed Dec 11 23:03:27 GMT 2024
On 12/6/24 8:29 AM, Yury Khrustalev wrote:
> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
>
> Free GCS after a makecontext start func returns and at thread exit, so
> assume makecontext cannot outlive the thread where it was created.
>
> This is an attempt to bound the lifetime of the GCS allocated for
> makecontext, but it is still possible to have significant GCS leaks,
> new GCS aware APIs could solve that, but that would not allow using
> GCS with existing code transparently.
We should avoid type-prone macro APIs and just define default versions of the
cleanup functions, and override it in aarch64. Avoiding the #ifdefs also reduces
bitrot over time and forces all other architectures to compile that code too.
> ---
> include/set-freeres.h | 4 ++
> malloc/thread-freeres.c | 3 +
> sysdeps/unix/sysv/linux/aarch64/makecontext.c | 65 +++++++++++++++++++
> sysdeps/unix/sysv/linux/aarch64/setcontext.S | 19 +++++-
> sysdeps/unix/sysv/linux/aarch64/sysdep.h | 6 +-
> 5 files changed, 93 insertions(+), 4 deletions(-)
>
> diff --git a/include/set-freeres.h b/include/set-freeres.h
> index 4177b453fa..c3d64b4f41 100644
> --- a/include/set-freeres.h
> +++ b/include/set-freeres.h
> @@ -78,6 +78,10 @@ extern void __nss_database_freeres (void) attribute_hidden;
> extern int _IO_cleanup (void) attribute_hidden;;
> /* From dlfcn/dlerror.c */
> extern void __libc_dlerror_result_free (void) attribute_hidden;
> +/* From libc.so, arch specific. */
> +#ifdef ARCH_THREAD_FREERES
> +extern void ARCH_THREAD_FREERES (void) attribute_hidden;
Typo prone, should always be defined.
> +#endif
>
> /* From either libc.so or libpthread.so */
> extern void __libpthread_freeres (void) attribute_hidden;
> diff --git a/malloc/thread-freeres.c b/malloc/thread-freeres.c
> index 55ba4e7b83..69867f3a3b 100644
> --- a/malloc/thread-freeres.c
> +++ b/malloc/thread-freeres.c
> @@ -29,6 +29,9 @@
> void
> __libc_thread_freeres (void)
> {
> +#ifdef ARCH_THREAD_FREERES
> + call_function_static_weak (ARCH_THREAD_FREERES);
> +#endif
This is typo prone.
We should just use "call_function_static_weak (ARCH_THREAD_FREERES);"
And define an empty ARCH_THREAD_FREERES in the base sysdep.h.
> #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_32)
> __rpc_thread_destroy ();
> #endif
> diff --git a/sysdeps/unix/sysv/linux/aarch64/makecontext.c b/sysdeps/unix/sysv/linux/aarch64/makecontext.c
> index 9e66b6761c..779f7e55aa 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/makecontext.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/makecontext.c
> @@ -20,7 +20,9 @@
> #include <sysdep.h>
> #include <stdarg.h>
> #include <stdint.h>
> +#include <stdlib.h>
> #include <ucontext.h>
> +#include <sys/mman.h>
>
> #define GCS_MAGIC 0x47435300
>
> @@ -29,6 +31,47 @@ static struct _aarch64_ctx *extension (void *p)
> return p;
> }
>
> +struct gcs_list {
> + struct gcs_list *next;
> + void *base;
> + size_t size;
> +};
> +
> +static __thread struct gcs_list *gcs_list_head = NULL;
> +
> +static void
> +record_gcs (void *base, size_t size)
> +{
> + struct gcs_list *p = malloc (sizeof *p);
> + if (p == NULL)
> + abort ();
> + p->base = base;
> + p->size = size;
> + p->next = gcs_list_head;
> + gcs_list_head = p;
> +}
> +
> +static void
> +free_gcs_list (void)
> +{
> + for (;;)
> + {
> + struct gcs_list *p = gcs_list_head;
> + if (p == NULL)
> + break;
> + gcs_list_head = p->next;
> + __munmap (p->base, p->size);
> + free (p);
> + }
> +}
> +
> +/* Called during thread shutdown to free resources. */
> +void
> +__libc_aarch64_thread_freeres (void)
> +{
> + free_gcs_list ();
> +}
> +
> #ifndef __NR_map_shadow_stack
> # define __NR_map_shadow_stack 453
> #endif
> @@ -58,6 +101,9 @@ alloc_makecontext_gcs (size_t stack_size)
> if (base == (void *) -1)
> /* ENOSYS, bad size or OOM. */
> abort ();
> +
> + record_gcs (base, size);
> +
> uint64_t *gcsp = (uint64_t *) ((char *) base + size);
> /* Skip end of GCS token. */
> gcsp--;
> @@ -69,6 +115,25 @@ alloc_makecontext_gcs (size_t stack_size)
> return gcsp + 1;
> }
>
> +void
> +__free_makecontext_gcs (void *gcs)
> +{
> + struct gcs_list *p = gcs_list_head;
> + struct gcs_list **q = &gcs_list_head;
> + for (;;)
> + {
> + if (p == NULL)
> + abort ();
> + if (gcs == p->base + p->size - 8)
> + break;
> + q = &p->next;
> + p = p->next;
> + }
> + *q = p->next;
> + __munmap (p->base, p->size);
> + free (p);
> +}
> +
> /* makecontext sets up a stack and the registers for the
> user context. The stack looks like this:
>
> diff --git a/sysdeps/unix/sysv/linux/aarch64/setcontext.S b/sysdeps/unix/sysv/linux/aarch64/setcontext.S
> index 3224c36c47..7a71ecf35f 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/setcontext.S
> +++ b/sysdeps/unix/sysv/linux/aarch64/setcontext.S
> @@ -34,6 +34,9 @@
> .text
>
> ENTRY (__setcontext)
> + /* If x10 is set then old GCS is freed. */
> + mov x10, 0
> +__setcontext_internal:
> PTR_ARG (0)
> /* Save a copy of UCP. */
> mov x9, x0
> @@ -145,7 +148,8 @@ ENTRY (__setcontext)
> ldr x3, [x2, #oGCSPR]
> MRS_GCSPR (x2)
> mov x4, x3
> - /* x2: GCSPR now. x3, x4: target GCSPR. x5, x6: tmp regs. */
> + mov x1, x2
> + /* x1, x2: GCSPR now. x3, x4: target GCSPR. x5, x6: tmp regs. */
> L(gcs_scan):
> cmp x2, x4
> b.eq L(gcs_pop)
> @@ -162,10 +166,18 @@ L(gcs_switch):
> GCSSS2 (xzr)
> L(gcs_pop):
> cmp x2, x3
> - b.eq L(gcs_done)
> + b.eq L(gcs_free_old)
> GCSPOPM (xzr)
> add x2, x2, 8
> b L(gcs_pop)
> +L(gcs_free_old):
> + cbz x10, L(gcs_done)
> + mov x28, x0
> + mov x0, x1
> + bl __free_makecontext_gcs
> + mov x0, x28
> + ldp x28, x29, [x0, oX0 + 28 * SZREG]
> + ldr x30, [x0, oX0 + 30 * SZREG]
> L(gcs_done):
>
> 2:
> @@ -186,6 +198,7 @@ ENTRY (__startcontext)
> cfi_undefined (x30)
> blr x20
> mov x0, x19
> - cbnz x0, __setcontext
> + mov x10, 1
> + cbnz x0, __setcontext_internal
> 1: b HIDDEN_JUMPTARGET (exit)
> END (__startcontext)
> diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> index 974b503b2f..97cf84eaaa 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> @@ -29,8 +29,12 @@
>
> #include <tls.h>
>
> -/* In order to get __set_errno() definition in INLINE_SYSCALL. */
> #ifndef __ASSEMBLER__
> +/* Thread cleanup function. */
> +#define ARCH_THREAD_FREERES __libc_aarch64_thread_freeres
OK. AArch64 definition.
> +void __libc_aarch64_thread_freeres (void) attribute_hidden;
> +
> +/* In order to get __set_errno() definition in INLINE_SYSCALL. */
> #include <errno.h>
> #endif
>
More information about the Libc-alpha
mailing list