This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ #11787] Fix stack guard size accounting
- From: Szabolcs Nagy <szabolcs dot nagy at arm dot com>
- To: Florian Weimer <fweimer at redhat dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Cc: nd at arm dot com
- Date: Wed, 20 Dec 2017 14:55:07 +0000
- Subject: Re: [PATCH][BZ #11787] Fix stack guard size accounting
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=Szabolcs dot Nagy at arm dot com;
- Nodisclaimer: True
- References: <5A2FE5ED.1080300@arm.com> <d84df8d6-efca-0a4e-605c-73defdb3ce93@redhat.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
On 20/12/17 13:09, Florian Weimer wrote:
> 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?
>
i can open a separate bug.
>> 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
>
makes sense.
but i saw several overflowing arithmetics around stack computation.