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

[Adhemerval Zanella Netto]

On 01/09/25 07:36, Yury Khrustalev wrote:
> 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.

My question is whether we need to save the 'shadow_stack_token' if we can
derive its address from shadow_stack_base itself.  It can be a arch-specific
function if required.

> 
>>> +  /* 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?

As far as I know, only the callstack resides on the guarded control stack, so 
there shouldn’t be any sensitive information stored there beyond what’s already 
present in the pthread cached stack. 

Since the thread cache only reinitializes the necessary fields (such as with 
`get_cached_stack`), reusing the shadow stack doesn’t expose any more data t
han what would already be available through stack reuse.
 > 
> 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.

There is currently no logic to resize the thread cache; it only adjusts the 
guarded control stack as needed. Remapping would defeat the cache’s main 
benefit, since allocating a new stack is typically faster (and, in cases 
where virtual memory area constraints are significant, users can already 
limit the cache size).

For guarded control stacks, I would expect each cache entry to store a 
pair—(stackblock_size, guarded_size)—so that, if a user specifies a different 
guarded control stack size, a new thread stack would be allocated. The chief 
advantage of the thread cache is that most `pthread_create` calls uses the 
default stack size, optimizing reuse.


As a side-note, things would be a lot more straightforward if userland could
simply allocate a region such as [stack + guard control stack], and the kernel 
offered an API (e.g., via `madvise`) to establish the control-guarded region. 
Alternatively, if `clone3` itself allowed definition of the guard region start, 
this could be exposed. My understanding is that the guarded region is intended 
to prevent all writes except specific GCS operations, effectively acting as a
guard page.

> 
>> 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?

I would say nptl, since this is Linux-specific anyway.