[PATCH v7 1/4] support: Add support_stack_alloc

[Florian Weimer]

* Adhemerval Zanella:

> On 07/07/2021 07:17, Florian Weimer wrote:
>> * Adhemerval Zanella via Libc-alpha:
>> 
>>> The code to allocate a stack from xsigstack is refactored so it can
>>> be more generic.  The new support_stack_alloc() also set PROT_EXEC
>>> if DEFAULT_STACK_PERMS has PF_X.  This is required on some
>>>  architectures (hppa for instance) and trying to access the rtld
>>> global from testsuite will require more intrusive refactoring
>>> in the ldsodefs.h header.
>> 
>> DEFAULT_STACK_PERMS is misnamed, it's really HISTORIC_STACK_PERMS.
>> All architectures override it to RW permissions in the toolchain
>> (maybe with the exception of Hurd, which uses trampolines for nested
>> functions).
>
> This is in fact two different requirements, this gnulib thread gives
> a nice summary about the permission required from trampolines [1]. 
> Another requirement is how Linux layout the signal return code for the 
> signal handler stack.  It seems that hppa still requires executable 
> stacks, since tst-xsigstack does fails without a executable stack even 
> on a recent 5.10.46-1 kernel.

Ugh, okay.

>> I have a cstack_allocate version that handles this.  It can only be done
>> from within glibc proper because we do not export the stack execution
>> status directly.  But I think it's out of scope for glibc 2.34 by now.
>
> We can in theory access the ldsodes.h fields directly and then
> use GL (dl_stack_flags) information to set the stack executable or not.
> The problem is ldsodefs.h is quite convoluted and it would require more
> refactoring to use outside libc.so code.  But I agree with you that
> having less hacky way to obtain this information is better.
>
> So are you ok with the current approach or being conservative and use
> DEFAULT_STACK_PERMS on libsupport?

DEFAULT_STACK_PERMS with a comment is fine.

I will resubmit my cstack_allocate patches for glibc 2.35 patches, and
they will fully handle executable stacks.

>>> +  /* The guard bands need to be large enough to intercept offset
>>> +     accesses from a stack address that might otherwise hit another
>>> +     mapping.  Make them at least twice as big as the stack itself, to
>>> +     defend against an offset by the entire size of a large
>>> +     stack-allocated array.  The minimum is 1MiB, which is arbitrarily
>>> +     chosen to be larger than any "typical" wild pointer offset.
>>> +     Again, no matter what the number is, round it up to a whole
>>> +     number of pages.  */
>>> +  size_t guardsize = roundup (MAX (2 * stacksize, 1024 * 1024), pagesize);
>>> +  size_t alloc_size = guardsize + stacksize + guardsize;
>>> +  /* Use MAP_NORESERVE so that RAM will not be wasted on the guard
>>> +     bands; touch all the pages of the actual stack before returning,
>>> +     so we know they are allocated.  */
>>> +  void *alloc_base = xmmap (0,
>>> +                            alloc_size,
>>> +                            PROT_NONE,
>>> +                            MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE|MAP_STACK,
>>> +                            -1);
>>> +  /* PF_X can be overridden if PT_GNU_STACK is present.  */
>>> +  int prot = PROT_READ | PROT_WRITE
>>> +	     | (DEFAULT_STACK_PERMS & PF_X ? PROT_EXEC : 0);
>>> +  xmprotect (alloc_base + guardsize, stacksize, prot);
>>> +  memset (alloc_base + guardsize, 0xA5, stacksize);
>>> +  return (struct support_stack) { alloc_base + guardsize, stacksize, guardsize };
>> 
>> This doesn't handle different stack growth directions.
>> 
>
> At least for the usages of the routine it does not require any adjustment:
> xsigaltstack and xclone will handle it.  I saw no regression for
> tst-xsigaltstack and tst-clone_range.

Huh, I would expect the guard area to be outside of the stack region
returned by stack allocation.  That's how the cstack_allocate API does
it.  If the current tests expect something else, then the approach in
the patch is okay (with a comment for DEFAULT_STACK_PERMS).

Thanks,
Florian