Re: [PATCH][BZ #11787] Fix stack guard size accounting

[Florian Weimer <fweimer at redhat dot com>]

On 12/12/2017 03:21 PM, Szabolcs Nagy wrote:
> Previously if user requested S stack and G guard when creating a
> thread, the total mapping was S and the actual available stack was
> S - G - static_tls, which is not what the user requested.
>
> This patch fixes the guard size accounting by pretending the user
> requested S + G stack.  This way all later logic works out except
> when reporting the user requested stack size (pthread_getattr_np)
> or when computing the minimal stack size (__pthread_get_minstack).
>
> Normally this will increase thread stack allocations by one page.
> TLS accounting is not affected, that will require a separate fix.

Should this fix use a separate bug number?

> 2017-12-12  Szabolcs Nagy<szabolcs.nagy@arm.com>
>
> 	[BZ #11787]
> 	* nptl/allocatestack.c (allocate_stack): Add guardsize to stacksize.
> 	* nptl/nptl-init.c (__pthread_get_minstack): Remove guardsize from
> 	stacksize.
> 	* nptl/pthread_getattr_np.c (pthread_getattr_np): Likewise.

Patch looks good in general.  The computation for stackblock_size looks 
right to me.

> diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
> index 1cc789319564b468cf07bdb1304b27dc5a91e86f..9525322b1f92bb34aa21dcab28566aecd7434e90 100644
> --- a/nptl/allocatestack.c
> +++ b/nptl/allocatestack.c
> @@ -532,6 +532,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
>         /* Make sure the size of the stack is enough for the guard and
>   	 eventually the thread descriptor.  */
>         guardsize = (attr->guardsize + pagesize_m1) & ~pagesize_m1;
> +      size += guardsize;
>         if (__builtin_expect (size < ((guardsize + __static_tls_size
>   				     + MINIMAL_REST_STACK + pagesize_m1)
>   				    & ~pagesize_m1),

I wonder if we should add an overflow check there and return EINVAL if

  guardsize < attr->guardsize || size + guardsize < guardsize

Thanks,
Florian