[PATCH v7 09/23] aarch64: Try to free the GCS of makecontext

[Yury Khrustalev]

On Tue, Jan 07, 2025 at 01:36:39PM -0300, Adhemerval Zanella Netto wrote:
> 
> 
> > +/* From libc.so, arch specific.  */
> > +extern void ARCH_THREAD_FREERES (void) attribute_hidden;
> >  
> 
> This seems strange if ARCH_THREAD_FREERES is not defined, although I am
> not sure if this is a problem.

I've been waiting for feedback from Carlos on this change. There is probably
a better way to do this without #ifdef but it's not obvious to me.

> > +/* Define empty function if no arch-specific clean-up
> > +   function has been defined.  */
> > +#ifndef ARCH_THREAD_FREERES
> > +void __always_inline
> > +__libc_arch_thread_freeres (void) {}
> > +#define ARCH_THREAD_FREERES __libc_arch_thread_freeres
> > +#endif
> > +
> 
> Do we need the __always_inline to force compiler to optimize it away?

Yes

> > @@ -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);
> > +
> 
> I think this will make makecontext non async-signal-safe when GCS is used,
> when we explict document it as AS-Safe and AC-Safe.  I think using
> internal_signal_block_all/internal_signal_restore_set would be suffice,
> but it is also a performance regression.

Do you refer to the call to malloc() in record_gcs()?

We need to keep track of shadow stacks allocated via map_shadow_stack()
when a new context is created with makecontext() so that we can munmap
it when it is no longer required.

I think this is not on a performance critical path, so we can add the
internal_signal_block_all / internal_signal_restore_set pair. Could you
recommend where it would be best to use it?

Thanks,
Yury