This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Add __private_ss to s390 struct tcbhead.
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Marcin KoÅcielnicki <koriakin at 0x04 dot net>, libc-alpha at sourceware dot org, Stefan Liebler <stli at linux dot vnet dot ibm dot com>, Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- Date: Wed, 13 Jan 2016 09:50:06 -0500
- Subject: Re: [PATCH] Add __private_ss to s390 struct tcbhead.
- Authentication-results: sourceware.org; auth=none
- References: <1451752466-9429-1-git-send-email-koriakin at 0x04 dot net> <5695B953 dot 9050807 at redhat dot com> <5696023A dot 6070405 at 0x04 dot net>
On 01/13/2016 02:52 AM, Marcin KoÅcielnicki wrote:
> On 13/01/16 03:41, Carlos O'Donell wrote:
>> On 01/02/2016 11:34 AM, Marcin KoÅcielnicki wrote:
>>> Preparation for gcc -fsplit-stack support (gcc bug #68191). The
>>> new field is basically identical to the one on x86. Its TCB
>>> offset needs to be constant, as it'll be hardcoded in gcc.
>>>
>>> * sysdeps/s390/nptl/tls.h: Add __private_ss to struct tcbhead.
>>
>> What happens if you run newly compiled code with split-stack
>> support on a glibc that doesn't include this space allocated in
>> tcbhead_t? You get a write beyond the tcbhead_t into some other
>> data? Depending on the thread memory layout that could be a guard
>> page or static TLS data?
>>
>> This is the same problem we saw in POWER when adding a field in
>> tcbhead_t for fast-architecture access.
>>
>> The only way to do this compatibly is to add a versioned symbol
>> that the compiler references to in order to prevent new binaries
>> from running on old glibc and crashing or worse silently corrupting
>> data.
>>
>> To reiterate: How do you plan to handle compatibility for this new
>> feature?
>>
>> If you did an analysis of nptl/allocatestack.c and showed that
>> there were still alignment bytes left, that might be one way to
>> work around this without needing a versioned reference.
>>
>
> I have added this field at the end of tcbhead_t, which is part of
> struct pthread, defined here:
> https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/descr.h;h=8e4938deb5311a325d6b67fbc3041037110c5f9e;hb=HEAD#l129
> . The tcbhead_t is stuffed there inside a union with void
> *__padding[24], which is much longer than current tcbhead_t. Thus
> applying this patch to glibc won't even change anything in compiler
> output - there's always been space there, and this patch merely
> reserves it for split-stack use.
Perfect. Sorry I missed that.
> The situation is different for POWER, since it has TLS_DTV_AT_TP - in
> this case, there's no padding for tcbhead_t and no extra expension
> space.
Correct.
Cheers,
Carlos.