[PATCH v2 2/3] nptl: Add libc allocated shadow stack for new threads

[Yury Khrustalev]

On Tue, Aug 26, 2025 at 04:07:25PM -0300, Adhemerval Zanella Netto wrote:
> 
> On 07/07/25 09:47, Yury Khrustalev wrote:
> > Includes aarch64 GCS support code, does not include x86 shstk
> > support (should be similar to the aarch64 case).
>  
> > diff --git a/nptl/descr.h b/nptl/descr.h
> > index ada6867a19..733e7b0ce6 100644
> > --- a/nptl/descr.h
> > +++ b/nptl/descr.h
> > @@ -384,6 +384,13 @@ struct pthread
> >    /* This is what the user specified and what we will report.  */
> >    size_t reported_guardsize;
> >  
> > +  /* Shadow stack base pointer.  */
> > +  void *shadow_stack_base;
> > +  /* Shadow stack architecture-defined token.  */
> > +  void *shadow_stack_token;
> 
> Isn't the shadow_stack_token derived from 'shadow_stack_base + shadow_stack_size - (2 * sizeof(void *)',
> since we allocate it with SHADOW_STACK_SET_TOKEN | SHADOW_STACK_SET_MARKER?

The token is derived in a target-specific way. In general, we need all 3
things at once: base and size for allocation / deallocation and token
for target-specific operations on shadow stacks.

> > +  /* Shadow stack size.  */
> > +  size_t shadow_stack_size;
> > +
> >    /* Thread Priority Protection data.  */
> >    struct priority_protection_data *tpp;
> >  
> > diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> > index e1033d4ee6..008e1f3684 100644
> > --- a/nptl/pthread_create.c
> > +++ b/nptl/pthread_create.c
> >
> > ...
> >
> >    pd->schedpolicy = self->schedpolicy;
> >    pd->schedparam = self->schedparam;
> >  
> > +  /* Set up a shadow stack.  */
> > +  err = THREAD_ALLOC_SHADOW_STACK (pd, stacksize);
> > +  if (__glibc_unlikely (err != 0))
> > +    {
> > +      __nptl_deallocate_stack (pd);
> > +      retval = err == ENOMEM ? EAGAIN : err;
> > +      goto out;
> > +    }
> > +
> 
> It adds a extra mmap (through map_shadow_stack) *and* munmap on every thread
> allocation; which kind clashes with the idea of thread stack cache.  Would
> it be possible to re-use a previous allocated shadow stack, so we can keep
> it allocated, but logical linked the map stack, so a pthread_create will
> just need to pop the stack from the cache and re-initialize the token?

This is possible, but I'd rather do it in a separate change. I need to
think about it. One negative implication about it would be the stale
data in the shadow stack that might somehow me used by an attacker (akin
use after free attack). We'd probably have to sanitise shadows stack
instead of unmapping them (and then re-initialise them properly), and
this might have even bigger overhead?

Also, in the future we might want to allow managing size of shadow
stacks including doing it individually for each thread. This would add
a step of looking for a stack of appropriate size in this cache.

> From kernel code it, it seems SHADOW_STACK_SET_TOKEN is just a matter of
> setup correct value from the shadow stack itself (with GCS_CAP_VALID_TOKEN)
> and issue a 'gcsb dsync'; but I am not fully sure.

> > +#ifdef HAVE_CLONE3_WRAPPER
> > +
> > +#define THREAD_ALLOC_SHADOW_STACK(pd, stacksize) \
> > +  thread_alloc_shadow_stack (pd, stacksize)
> 
> Should we also name the shadow stacks with name_stack_maps (if possible)?

I'll need to look into this. I wasn't aware of this API.

> > +static inline int
> > +__clone3_supports_shadow_stack (void)
> > +{
> > +  struct clone_args args = {
> > +  /* This guarantees that no thread will actually be created due to
> > +     strict alignment requirements for the shadow stack token.  */
> > +    .shadow_stack_token = 1,
> > +  };
> > +  /* Size that is required specifically for the shadow_stack_token
> > +     field of the struct clone_args type.  */
> > +  const size_t sz = offsetofend (struct clone_args, shadow_stack_token);
> > +  int saved_errno = errno;
> > +  /* Function passed to clone3 must be non-null otherwise we will
> > +     get EINVAL from the Glibc wrapper rather than the kernel.  */
> > +  int ret = __clone3 (&args, sz, (int (*)(void *))1, NULL);
> > +  if (ret == -1 && errno == E2BIG)
> > +    ret = 1;
> > +  else
> > +    ret = 2;
> 
> I think we should have an enum here, instead of magic constants.

Good point, will do.

> > +  __set_errno (saved_errno);
> > +  return ret;
> > +}
> > +
> > +static inline int
> > +thread_alloc_shadow_stack (struct pthread *pd, size_t stacksize)
> > +{
> > +  if (!has_gcs ())
> > +    return 0;
> > +  /* Support for shadow stack token in clone3 args:
> > +       0 - not checked,
> > +       1 - checked, not supported,
> > +       2 - checked, supported.  */
> > +  static int clone3_has_shadow_stack = 0;
> > +  if (atomic_load_relaxed (&clone3_has_shadow_stack) == 0)
> > +    {
> > +      int res = __clone3_supports_shadow_stack();
> > +      atomic_store_relaxed (&clone3_has_shadow_stack, res);
> > +    }
> > +  if (atomic_load_relaxed (&clone3_has_shadow_stack) == 1)
> > +    return 0;
> > +  struct gcs_record gcs;
> > +  if (__alloc_gcs (stacksize, &gcs) != NULL)
> > +    {
> > +      pd->shadow_stack_base = gcs.gcs_base;
> > +      pd->shadow_stack_token = gcs.gcs_token;
> > +      pd->shadow_stack_size = gcs.gcs_size;
> > +    }
> > +  /* Ignore errors: leave GCS allocation to the kernel.  */
> > +  return 0;
> > +}
> > +
> 
> Afaiu the clone3 check should be platform neutral; although the shadow
> stack creation requires platform specific considerations.  Maybe move the
> clone3 to generic support, since I take x86 (and maybe RISCV in the future)
> will most likely to use the same support.

OK, this is probably a good idea, although this would be an unused
symbol or all other targets. Where should I put the generic check
function in the source tree?

Thanks,
Yury