[RFC] nptl: change default stack guard size of threads

classic Classic list List threaded Threaded
42 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[RFC] nptl: change default stack guard size of threads

Szabolcs Nagy-2
RFC patch based on discussions about stack probing at
https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02306.html

0001-nptl-change-default-stack-guard-size-of-threads.patch (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Florian Weimer-5
On 11/29/2017 03:59 PM, Szabolcs Nagy wrote:
> The change can be made for aarch64 only

That doesn't seem to be the case, looking at the patch.

So what you intended to do, exactly?

A 64 KiB probe interval on legacy 32-bit architectures is really a
no-go.  It means we have to increase the guard region size to 64 KiB.
But we cannot do that: The guard allocation comes out of the overall
thread stack size, and existing applications do not expect that 60K of
configured stack suddenly becomes unavailable.  Adding the guard size on
top of the allocation will break setups which are carefully tuned for a
maximum number of threads.

And that's what people actually do.  Here's an example:


  -Xss128k

Reduces the default maximum thread stack size, which allows more of the
process' virtual memory address space to be used by the Java heap.


<http://www.oracle.com/technetwork/java/tuning-139912.html>

We can likely support 64 KiB probe intervals on 64-bit architectures.
But given the impact on backwards compatibility, I really don't see the
benefit on (legacy) 32-bit.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Carlos O'Donell-6
On 11/29/2017 07:18 AM, Florian Weimer wrote:

> On 11/29/2017 03:59 PM, Szabolcs Nagy wrote:
>> The change can be made for aarch64 only
>
> That doesn't seem to be the case, looking at the patch.
>
> So what you intended to do, exactly?
>
> A 64 KiB probe interval on legacy 32-bit architectures is really a
> no-go.  It means we have to increase the guard region size to 64 KiB.
> But we cannot do that: The guard allocation comes out of the overall
> thread stack size, and existing applications do not expect that 60K
> of configured stack suddenly becomes unavailable.  Adding the guard
> size on top of the allocation will break setups which are carefully
> tuned for a maximum number of threads.

We cannot be held to account for carefully tuned applications, such
applications have to be tuned again for newer glibc.

I think we *could* do this for 64-bit and 32-bit AArch64/ARM, but I
don't see the value in doing it for 32-bit.

> And that's what people actually do.  Here's an example:
>
> “
>  -Xss128k
>
> Reduces the default maximum thread stack size, which allows more of the process' virtual memory address space to be used by the Java heap.
> ”
>
> <http://www.oracle.com/technetwork/java/tuning-139912.html>
>
> We can likely support 64 KiB probe intervals on 64-bit architectures.
> But given the impact on backwards compatibility, I really don't see
> the benefit on (legacy) 32-bit.

I agree, this is expensive for 32-bit, without much reward.

Even on 64-bit, I would like to see bug 11787 fixed to move the guard
page accounting out of the stack, and then you can make the guard page
as big as you want without impacting the stack accounting.

--
Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Rich Felker-2
On Wed, Nov 29, 2017 at 10:16:54AM -0800, Carlos O'Donell wrote:

> On 11/29/2017 07:18 AM, Florian Weimer wrote:
> > On 11/29/2017 03:59 PM, Szabolcs Nagy wrote:
> >> The change can be made for aarch64 only
> >
> > That doesn't seem to be the case, looking at the patch.
> >
> > So what you intended to do, exactly?
> >
> > A 64 KiB probe interval on legacy 32-bit architectures is really a
> > no-go.  It means we have to increase the guard region size to 64 KiB.
> > But we cannot do that: The guard allocation comes out of the overall
> > thread stack size, and existing applications do not expect that 60K
> > of configured stack suddenly becomes unavailable.  Adding the guard
> > size on top of the allocation will break setups which are carefully
> > tuned for a maximum number of threads.
>
> We cannot be held to account for carefully tuned applications, such
> applications have to be tuned again for newer glibc.
>
> I think we *could* do this for 64-bit and 32-bit AArch64/ARM, but I
> don't see the value in doing it for 32-bit.

If 64k guard is mandatory for safety against jumping over the guard
zone, then I don't think it's possible to "re-tune" 32-bit apps for
the new requirement. This imposes a relatively small limit on possible
number of threads the process can create.

> > And that's what people actually do.  Here's an example:
> >
> > “
> >  -Xss128k
> >
> > Reduces the default maximum thread stack size, which allows more of the process' virtual memory address space to be used by the Java heap.
> > ”
> >
> > <http://www.oracle.com/technetwork/java/tuning-139912.html>
> >
> > We can likely support 64 KiB probe intervals on 64-bit architectures.
> > But given the impact on backwards compatibility, I really don't see
> > the benefit on (legacy) 32-bit.
>
> I agree, this is expensive for 32-bit, without much reward.
>
> Even on 64-bit, I would like to see bug 11787 fixed to move the guard
> page accounting out of the stack, and then you can make the guard page
> as big as you want without impacting the stack accounting.

I agree completely that guard page should not subtract from the usable
stack size but should be in addition to it. If glibc is not currently
behaving that way, I think it's a conformance bug. But making it big
on 32-bit quickly cuts into total available virtual address space if
you have a moderately large number of threads.

Rich
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Szabolcs Nagy-2
In reply to this post by Florian Weimer-5
On 29/11/17 15:18, Florian Weimer wrote:
> On 11/29/2017 03:59 PM, Szabolcs Nagy wrote:
>> The change can be made for aarch64 only
>
> That doesn't seem to be the case, looking at the patch.
>
> So what you intended to do, exactly?

it seems the issue applies to all targets, and since
glibc itself have 64k stack jumps i sent the rfc
patch for all targets to err on the safe side.

some targets will have 4k probe interval in gcc but
even those are not safe with existing binaries and
several targets have no proposed stack probe patch
as far as i understand.

> A 64 KiB probe interval on legacy 32-bit architectures is really a no-go.  It means we have to increase the
> guard region size to 64 KiB. But we cannot do that: The guard allocation comes out of the overall thread stack
> size, and existing applications do not expect that 60K of configured stack suddenly becomes unavailable.
> Adding the guard size on top of the allocation will break setups which are carefully tuned for a maximum number
> of threads.

i was aware of the address space limitation on 32bit
but e.g. aarch64 ilp32 will need the 64k guardsize too.

(i think there are other 32bit targets that support
>4k page size, those may not mind the increase either,
they have to portably support large page size anyway)

i was not aware of bug 11787 and i agree that should
be fixed before the default guardsize is increased.

ARCH_GUARD_DEFAULT_SIZE is a macro that each target
can override so it can be set to 4k on legacy 32bit
targets or its default value can be changed for
__WORDSIZE == 32.

on aarch64 at least the default guard size should be
increased but it may even be better to enforce a minimum
64k guardsize if it is set to a non-zero value by the user.
i was not sure if that's acceptable (observably changing
what the user requested) hence this patch only touches
the default size.

this has implications to generic glibc behaviour
and that's where i'd like to see feedback, because
if there are issues then we may need to rethink
the aarch64 probing implementation in gcc.

e.g. user allocated stacks (thread or signal) cannot
be fixed, so if it is known common practice to allocate
those with single (or no) guard page then changing
glibc default does not solve the problem.

Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Florian Weimer-5
In reply to this post by Rich Felker-2
On 11/29/2017 07:29 PM, Rich Felker wrote:

> On Wed, Nov 29, 2017 at 10:16:54AM -0800, Carlos O'Donell wrote:
>> On 11/29/2017 07:18 AM, Florian Weimer wrote:
>>> On 11/29/2017 03:59 PM, Szabolcs Nagy wrote:
>>>> The change can be made for aarch64 only
>>>
>>> That doesn't seem to be the case, looking at the patch.
>>>
>>> So what you intended to do, exactly?
>>>
>>> A 64 KiB probe interval on legacy 32-bit architectures is really a
>>> no-go.  It means we have to increase the guard region size to 64 KiB.
>>> But we cannot do that: The guard allocation comes out of the overall
>>> thread stack size, and existing applications do not expect that 60K
>>> of configured stack suddenly becomes unavailable.  Adding the guard
>>> size on top of the allocation will break setups which are carefully
>>> tuned for a maximum number of threads.
>>
>> We cannot be held to account for carefully tuned applications, such
>> applications have to be tuned again for newer glibc.
>>
>> I think we *could* do this for 64-bit and 32-bit AArch64/ARM, but I
>> don't see the value in doing it for 32-bit.
>
> If 64k guard is mandatory for safety against jumping over the guard
> zone, then I don't think it's possible to "re-tune" 32-bit apps for
> the new requirement. This imposes a relatively small limit on possible
> number of threads the process can create.

With the probing implementations I have seen so far, it is feasible
technically because a probe will never touch a stack area which could
not conceivably be touched by a signal handler, too.

But it is still a bad situation when GCC documents that it is only
providing full protection with guard size X, and you decide to run with
size X/16 to actually get things going.  This means that your
configuration is out of scope what your vendor will support
security-wise, and probably non-compliant with applicable guidelines at
the user site.

So technically you can run with a smaller guard size, but it's still
impractical to do so for most organizations.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Florian Weimer-5
In reply to this post by Szabolcs Nagy-2
On 11/29/2017 07:40 PM, Szabolcs Nagy wrote:

> On 29/11/17 15:18, Florian Weimer wrote:
>> On 11/29/2017 03:59 PM, Szabolcs Nagy wrote:
>>> The change can be made for aarch64 only
>>
>> That doesn't seem to be the case, looking at the patch.
>>
>> So what you intended to do, exactly?
>
> it seems the issue applies to all targets, and since
> glibc itself have 64k stack jumps i sent the rfc
> patch for all targets to err on the safe side.

glibc has many arbitrarily large stack jumps (although we've been
eliminating them manually for a while).

What should guide the default size of the guard is not what glibc needs
for its own routines, but what the stack probing in GCC needs to be correct.

> some targets will have 4k probe interval in gcc but
> even those are not safe with existing binaries and
> several targets have no proposed stack probe patch
> as far as i understand.

Isn't there are generic variant which covers at least alloca and VLAs?

>> A 64 KiB probe interval on legacy 32-bit architectures is really a no-go.  It means we have to increase the
>> guard region size to 64 KiB. But we cannot do that: The guard allocation comes out of the overall thread stack
>> size, and existing applications do not expect that 60K of configured stack suddenly becomes unavailable.
>> Adding the guard size on top of the allocation will break setups which are carefully tuned for a maximum number
>> of threads.
>
> i was aware of the address space limitation on 32bit
> but e.g. aarch64 ilp32 will need the 64k guardsize too.

Why?

This is a new feature.  Why make this less usable from the start?

(I don't care about aarc64 ILP32 and page sizes smaller than 64 KiB on
aarch64 in general, so I wont argue this, and this is just a courtesy
notification that what you are doing is Very Wrong Indeed.)

> (i think there are other 32bit targets that support
> > 4k page size, those may not mind the increase either,
> they have to portably support large page size anyway)

GCC needs to emit probe intervals for the smallest supported page size
on the the target architecture.  If it does not do that, we end up in
trouble on the glibc side.

We can throw new code at this problem and solve it for 64-bit.  For
32-bit, we simply do not have a universally applicable solution.  My
understanding was that everywhere except on ARM, GCC was compatible with
the pioneering glibc/Linux work in this area (the guard page we added to
thread stacks, and the guard page added by the kernel).  If this isn't
the case, then I'm really disappointed in the disregard of existing
practice on the GCC side.

> e.g. user allocated stacks (thread or signal) cannot
> be fixed, so if it is known common practice to allocate
> those with single (or no) guard page then changing
> glibc default does not solve the problem.

We have test cases which do not allocate a guard page for those, but
with the existing probing algorithms I've seen, this is not actually a
problem if stack usage is kept low.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Rich Felker-2
On Wed, Nov 29, 2017 at 09:44:14PM +0100, Florian Weimer wrote:

> On 11/29/2017 07:40 PM, Szabolcs Nagy wrote:
> >On 29/11/17 15:18, Florian Weimer wrote:
> >>On 11/29/2017 03:59 PM, Szabolcs Nagy wrote:
> >>>The change can be made for aarch64 only
> >>
> >>That doesn't seem to be the case, looking at the patch.
> >>
> >>So what you intended to do, exactly?
> >
> >it seems the issue applies to all targets, and since
> >glibc itself have 64k stack jumps i sent the rfc
> >patch for all targets to err on the safe side.
>
> glibc has many arbitrarily large stack jumps (although we've been
> eliminating them manually for a while).
>
> What should guide the default size of the guard is not what glibc
> needs for its own routines, but what the stack probing in GCC needs
> to be correct.

Agreed.

> >>A 64 KiB probe interval on legacy 32-bit architectures is really a no-go.  It means we have to increase the
> >>guard region size to 64 KiB. But we cannot do that: The guard allocation comes out of the overall thread stack
> >>size, and existing applications do not expect that 60K of configured stack suddenly becomes unavailable.
> >>Adding the guard size on top of the allocation will break setups which are carefully tuned for a maximum number
> >>of threads.
> >
> >i was aware of the address space limitation on 32bit
> >but e.g. aarch64 ilp32 will need the 64k guardsize too.
>
> Why?
>
> This is a new feature.  Why make this less usable from the start?
>
> (I don't care about aarc64 ILP32 and page sizes smaller than 64 KiB
> on aarch64 in general, so I wont argue this, and this is just a
> courtesy notification that what you are doing is Very Wrong Indeed.)

I'm not sure I follow, but from the standpoint of virtual address
space and what is an acceptable cost in wasted address space, any
ILP32-on-64 ABIs should be considered the same as 32-bit archs. As
such, I think GCC really needs to do the stack probe every 4k, not
64k, and the default (and certainly minimum-supported) guard size
should be kept at 4k, not 64k or anything larger.

> >(i think there are other 32bit targets that support
> >> 4k page size, those may not mind the increase either,
> >they have to portably support large page size anyway)
>
> GCC needs to emit probe intervals for the smallest supported page
> size on the the target architecture.  If it does not do that, we end
> up in trouble on the glibc side.

Agreed.

> We can throw new code at this problem and solve it for 64-bit.  For
> 32-bit, we simply do not have a universally applicable solution.  My
> understanding was that everywhere except on ARM, GCC was compatible
> with the pioneering glibc/Linux work in this area (the guard page we
> added to thread stacks, and the guard page added by the kernel).  If
> this isn't the case, then I'm really disappointed in the disregard
> of existing practice on the GCC side.

Hm? What are you thinking of that GCC might have gotten wrong?

Rich
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Florian Weimer-5
On 11/29/2017 09:51 PM, Rich Felker wrote:

> I'm not sure I follow, but from the standpoint of virtual address
> space and what is an acceptable cost in wasted address space, any
> ILP32-on-64 ABIs should be considered the same as 32-bit archs. As
> such, I think GCC really needs to do the stack probe every 4k, not
> 64k, and the default (and certainly minimum-supported) guard size
> should be kept at 4k, not 64k or anything larger.

Yes, and I expect that we will keep using 4 KiB probing on i386 (and
s390/s390x).  That's what Jeff gave me for testing.  I do hope the final
upstream version isn't going to be different in this regard.

But in the end, this is up to the machine maintainers (for gcc and glibc).

>> We can throw new code at this problem and solve it for 64-bit.  For
>> 32-bit, we simply do not have a universally applicable solution.  My
>> understanding was that everywhere except on ARM, GCC was compatible
>> with the pioneering glibc/Linux work in this area (the guard page we
>> added to thread stacks, and the guard page added by the kernel).  If
>> this isn't the case, then I'm really disappointed in the disregard
>> of existing practice on the GCC side.
>
> Hm? What are you thinking of that GCC might have gotten wrong?

Use 64 KiB probe intervals (almost) everywhere as an optimization.  I
assumed the original RFC patch was motivated by that.

I knew that ARM would be broken because that's what the gcc ARM
maintainers want.  I assumed that it was restricted to that, but now I'm
worried that it's not.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Wilco Dijkstra-2
In reply to this post by Florian Weimer-5
Florian Weimer wrote:

> glibc has many arbitrarily large stack jumps (although we've been
> eliminating them manually for a while).
>
> What should guide the default size of the guard is not what glibc needs
> for its own routines, but what the stack probing in GCC needs to be correct.

It's not related to what GLIBC needs, but GLIBC, like Linux, must continue to
run old binaries so a larger guard size is definitely beneficial. No existing code
uses probing, so increasing the guard size means far fewer functions could
jump the guard.  The dropoff is exponential, each doubling of guard page size
halves the number of functions with a stack larger than the guard size. That's
why Linux went for a 1MB guard size. A larger thread guard size could be the
default on 64-bit targets or a per-target choice depending on whether they
want to improve safety.

> > i was aware of the address space limitation on 32bit
> > but e.g. aarch64 ilp32 will need the 64k guardsize too.
>
> Why?
>
> This is a new feature.  Why make this less usable from the start?

Why should it be any different from LP64? Typical page size will still be
64KB, so a 4KB guard would be rounded up to 64KB. An ILP32 system
with 64KB pages could create  ~30000 threads per process. Does that
make it unusable?

> GCC needs to emit probe intervals for the smallest supported page size
> on the the target architecture.  If it does not do that, we end up in
> trouble on the glibc side.

Smallest supported page size on Arm is 1KB. Should that dictate
everything? It's an ABI choice, not something directly related to
smallest page size.

> > e.g. user allocated stacks (thread or signal) cannot
> > be fixed, so if it is known common practice to allocate
> > those with single (or no) guard page then changing
> > glibc default does not solve the problem.
>
> We have test cases which do not allocate a guard page for those, but
> with the existing probing algorithms I've seen, this is not actually a
> problem if stack usage is kept low.

Probing without a guard page is equivalent to no probing. So for these
cases we're simply unprotected.

Wilco
   
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Carlos O'Donell-6
On 11/29/2017 02:28 PM, Wilco Dijkstra wrote:
> Why should it be any different from LP64? Typical page size will still be
> 64KB, so a 4KB guard would be rounded up to 64KB. An ILP32 system
> with 64KB pages could create  ~30000 threads per process. Does that
> make it unusable?

... and this is already configurable via pthread_attr_setguardsize(), so
you *could* tune a system to have smaller guards against the recommendation
of the runtime authors?

--
Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Szabolcs Nagy
In reply to this post by Florian Weimer-5
* Florian Weimer <[hidden email]> [2017-11-29 21:44:14 +0100]:

> On 11/29/2017 07:40 PM, Szabolcs Nagy wrote:
> > On 29/11/17 15:18, Florian Weimer wrote:
> > > On 11/29/2017 03:59 PM, Szabolcs Nagy wrote:
> > > > The change can be made for aarch64 only
> > >
> > > That doesn't seem to be the case, looking at the patch.
> > >
> > > So what you intended to do, exactly?
> >
> > it seems the issue applies to all targets, and since
> > glibc itself have 64k stack jumps i sent the rfc
> > patch for all targets to err on the safe side.
>
> glibc has many arbitrarily large stack jumps (although we've been
> eliminating them manually for a while).
>
> What should guide the default size of the guard is not what glibc needs for
> its own routines, but what the stack probing in GCC needs to be correct.
>

assuming you dont care about legacy binaries or binaries
where probing was turned off.

> > some targets will have 4k probe interval in gcc but
> > even those are not safe with existing binaries and
> > several targets have no proposed stack probe patch
> > as far as i understand.
>
> Isn't there are generic variant which covers at least alloca and VLAs?
>
> > > A 64 KiB probe interval on legacy 32-bit architectures is really a no-go.  It means we have to increase the
> > > guard region size to 64 KiB. But we cannot do that: The guard allocation comes out of the overall thread stack
> > > size, and existing applications do not expect that 60K of configured stack suddenly becomes unavailable.
> > > Adding the guard size on top of the allocation will break setups which are carefully tuned for a maximum number
> > > of threads.
> >
> > i was aware of the address space limitation on 32bit
> > but e.g. aarch64 ilp32 will need the 64k guardsize too.
>
> Why?
>
> This is a new feature.  Why make this less usable from the start?
>
> (I don't care about aarc64 ILP32 and page sizes smaller than 64 KiB on
> aarch64 in general, so I wont argue this, and this is just a courtesy
> notification that what you are doing is Very Wrong Indeed.)
>
> > (i think there are other 32bit targets that support
> > > 4k page size, those may not mind the increase either,
> > they have to portably support large page size anyway)
>
> GCC needs to emit probe intervals for the smallest supported page size on
> the the target architecture.  If it does not do that, we end up in trouble
> on the glibc side.
>
> We can throw new code at this problem and solve it for 64-bit.  For 32-bit,
> we simply do not have a universally applicable solution.  My understanding
> was that everywhere except on ARM, GCC was compatible with the pioneering
> glibc/Linux work in this area (the guard page we added to thread stacks, and
> the guard page added by the kernel).  If this isn't the case, then I'm
> really disappointed in the disregard of existing practice on the GCC side.

i was not involved in the gcc probing design, but i noticed
the glibc implications that's why i posted a patch, my
understanding is that the efficiency of the probing on a
target depends on

- is there an implicit probe already at calls? (return
address stored on stack vs in register) if not then a
function using the stack must make an assumption about
how far the previous probe might have been (this is a
new call abi)
- is there a way to atomically increment sp and probe it
or access memory below sp?  if not then sp can only be
incremented by guardsize-eps at a time where eps depends
on how signal frame is stored. (and if signal frame is
large and written the wrong way then it may be impossible
to avoid memory writes accross a guard at all times)
- frame layout (incomming/outgoing args, locals, frame
pointer etc) can make probing require significant changes
to function prologues (extra instructinos) or not.

on targets like aarch64 the most conservative design
might have enough performance impact to make users turn
it off.

ilp32 has the same probing design as lp64 so it requires
the same guardsize.

to me it seemed a tradeoff between performance overhead
vs address-space overhead, if there are runtime troubles
because of the larger guardsize requirement then the
tradeoff may need to be revisited.
(with better data on both sides)

i also didnt think 64k guard would be prohibitive on
ilp32 (i know there are usecases where it is a problem,
but i expected those to be very rare cases)
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Rich Felker-2
In reply to this post by Wilco Dijkstra-2
On Wed, Nov 29, 2017 at 10:28:49PM +0000, Wilco Dijkstra wrote:

> > > i was aware of the address space limitation on 32bit
> > > but e.g. aarch64 ilp32 will need the 64k guardsize too.
> >
> > Why?
> >
> > This is a new feature.  Why make this less usable from the start?
>
> Why should it be any different from LP64? Typical page size will still be
> 64KB, so a 4KB guard would be rounded up to 64KB. An ILP32 system
> with 64KB pages could create  ~30000 threads per process. Does that
> make it unusable?

A large page size like 64k seriously impacts the usefulness of a
32-bit virtual address space. You're limited to at most 64k total
pages (assuming userspace can use the whole 4GB which is likely if
it's 64-bit kernel/hardware, but not if it's 32-bit), essentially zero
bits of randomness for ASLR, etc. I understand that there's a trend to
favor larger page sizes to mitigate TLB pressure in huge software; I
don't think there's been nearly enough consideration of the negative
consequences of this trend.

> > GCC needs to emit probe intervals for the smallest supported page size
> > on the the target architecture.  If it does not do that, we end up in
> > trouble on the glibc side.
>
> Smallest supported page size on Arm is 1KB. Should that dictate
> everything? It's an ABI choice, not something directly related to
> smallest page size.

For the purposes of this discussion, hardware-supported page sizes
that are not compatible with the ABI page size are not relevant. For
example SYS_mmap2 cannot represent 1k pages so they're not a
possibility with the ABI. Also, runtime page sizes larger than the ELF
segment alignment are not compatible with the ABI.

Rich
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Szabolcs Nagy
In reply to this post by Florian Weimer-5
* Florian Weimer <[hidden email]> [2017-11-29 22:02:48 +0100]:
> On 11/29/2017 09:51 PM, Rich Felker wrote:
> > Hm? What are you thinking of that GCC might have gotten wrong?
>
> Use 64 KiB probe intervals (almost) everywhere as an optimization.  I
> assumed the original RFC patch was motivated by that.
>
> I knew that ARM would be broken because that's what the gcc ARM maintainers
> want.  I assumed that it was restricted to that, but now I'm worried that
> it's not.

no,
it was just much easier to post the patch without setting a
value for each target separately, i believe that larger guard
is useful on all 64bit targets, but to make the patch depend
on __WORDSIZE is also wrong since i knew i'd need different
setting for aarch64 ilp32
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

James Greenhalgh-2
In reply to this post by Florian Weimer-5
On Wed, Nov 29, 2017 at 09:02:48PM +0000, Florian Weimer wrote:

> On 11/29/2017 09:51 PM, Rich Felker wrote:
>
> > I'm not sure I follow, but from the standpoint of virtual address
> > space and what is an acceptable cost in wasted address space, any
> > ILP32-on-64 ABIs should be considered the same as 32-bit archs. As
> > such, I think GCC really needs to do the stack probe every 4k, not
> > 64k, and the default (and certainly minimum-supported) guard size
> > should be kept at 4k, not 64k or anything larger.
>
> Yes, and I expect that we will keep using 4 KiB probing on i386 (and
> s390/s390x).  That's what Jeff gave me for testing.  I do hope the final
> upstream version isn't going to be different in this regard.
>
> But in the end, this is up to the machine maintainers (for gcc and glibc).
>
> >> We can throw new code at this problem and solve it for 64-bit.  For
> >> 32-bit, we simply do not have a universally applicable solution.  My
> >> understanding was that everywhere except on ARM, GCC was compatible
> >> with the pioneering glibc/Linux work in this area (the guard page we
> >> added to thread stacks, and the guard page added by the kernel).  If
> >> this isn't the case, then I'm really disappointed in the disregard
> >> of existing practice on the GCC side.
> >
> > Hm? What are you thinking of that GCC might have gotten wrong?
>
> Use 64 KiB probe intervals (almost) everywhere as an optimization.  I
> assumed the original RFC patch was motivated by that.
>
> I knew that ARM would be broken because that's what the gcc ARM
> maintainers want.  I assumed that it was restricted to that, but now I'm
> worried that it's not.

To be clear here, I'm coming in to the review of the probing support in GCC
late, and with very little context on the design of the feature. I certainly
wouldn't want to cause you worry - I've got no intention of pushing for
optimization to a larger guard page size if it would leaves things broken
for AArch64.

Likewise, I have no real desire for us to emit a bunch of extra operations
if we're not required to for glibc.

If I'm reopening earlier conversations, it is only because I wasn't involved
in them. I have no interest in us doing something "Very Wrong Indeed".

If assuming that 64k probes are sufficient on AArch64 is not going to allow
us a correct implementation, then we can't assume 64k probes on AArch64. My
understanding was that we were safe in this as the kernel was giving us a
generous 1MB to play with, and we could modify glibc to also give us 64k
(I admit, I had not considered ILP32, where you've rightly pointed out we
will eat lots of address space if we make this decision).

> > GCC needs to emit probe intervals for the smallest supported page size
> > on the the target architecture.  If it does not do that, we end up in
> > trouble on the glibc side.

This is where I may have a misunderstanding, why would it require probing
at the smallest page size, rather than probing at a multiple of the guard
size? It is very likely I'm missing something here as I don't know the glibc
side of this at all.

Thanks for your advice so far. To reiterate, I'm not pushing any particular
optimization agenda in GCC, but I would like to understand the trade-off
we're discussing.

James

Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Florian Weimer-5
On 12/05/2017 11:55 AM, James Greenhalgh wrote:

> To be clear here, I'm coming in to the review of the probing support in GCC
> late, and with very little context on the design of the feature. I certainly
> wouldn't want to cause you worry - I've got no intention of pushing for
> optimization to a larger guard page size if it would leaves things broken
> for AArch64.

The situation with aarch64 is perhaps a bit complicated because we are
faced with different kernel builds with different page sizes (4 KiB on
the low end, 64 KiB for enterprise workloads).

> If assuming that 64k probes are sufficient on AArch64 is not going to allow
> us a correct implementation, then we can't assume 64k probes on AArch64. My
> understanding was that we were safe in this as the kernel was giving us a
> generous 1MB to play with, and we could modify glibc to also give us 64k
> (I admit, I had not considered ILP32, where you've rightly pointed out we
> will eat lots of address space if we make this decision).
You can't modify existing glibc installations which use one page for the
guard.  For a 32-bit address space, I hope the consensus is that we
don't want to switch to multiple guard pages by default.

All this means that it will be very difficult to document the protection
afforded by the -fstack-clash-protection option.

This problem goes away completely if you probe at intervals of the
minimum page size the system supports because existing glibc versions
will interoperate with that (in the sense that they provide the intended
level of protection).

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Florian Weimer-5
In reply to this post by Carlos O'Donell-6
On 11/29/2017 11:38 PM, Carlos O'Donell wrote:
> On 11/29/2017 02:28 PM, Wilco Dijkstra wrote:
>> Why should it be any different from LP64? Typical page size will still be
>> 64KB, so a 4KB guard would be rounded up to 64KB. An ILP32 system
>> with 64KB pages could create  ~30000 threads per process. Does that
>> make it unusable?
>
> ... and this is already configurable via pthread_attr_setguardsize(), so
> you *could* tune a system to have smaller guards against the recommendation
> of the runtime authors?

Sure, if you recompile your application.  That's not really a solution IMHO.

Florian
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Wilco Dijkstra-2
Florian Weimer wrote:

> On 11/29/2017 11:38 PM, Carlos O'Donell wrote:
>> On 11/29/2017 02:28 PM, Wilco Dijkstra wrote:
>>> Why should it be any different from LP64? Typical page size will still be
>>> 64KB, so a 4KB guard would be rounded up to 64KB. An ILP32 system
>>> with 64KB pages could create  ~30000 threads per process. Does that
>>> make it unusable?
>>
>> ... and this is already configurable via pthread_attr_setguardsize(), so
>> you *could* tune a system to have smaller guards against the recommendation
>> of the runtime authors?
>
> Sure, if you recompile your application.  That's not really a solution IMHO.

You've got to do that already in order to run it on ILP32 - there isn't exactly
a large amount of existing ILP32 binaries around, let alone ones that need more
than 30000 threads per process.

Wilco
   
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Florian Weimer-5
On 12/06/2017 02:10 PM, Wilco Dijkstra wrote:

> Florian Weimer wrote:
>> On 11/29/2017 11:38 PM, Carlos O'Donell wrote:
>>> On 11/29/2017 02:28 PM, Wilco Dijkstra wrote:
>>>> Why should it be any different from LP64? Typical page size will still be
>>>> 64KB, so a 4KB guard would be rounded up to 64KB. An ILP32 system
>>>> with 64KB pages could create  ~30000 threads per process. Does that
>>>> make it unusable?
>>>
>>> ... and this is already configurable via pthread_attr_setguardsize(), so
>>> you *could* tune a system to have smaller guards against the recommendation
>>> of the runtime authors?
>>
>> Sure, if you recompile your application.  That's not really a solution IMHO.
>
> You've got to do that already in order to run it on ILP32 - there isn't exactly
> a large amount of existing ILP32 binaries around, let alone ones that need more
> than 30000 threads per process.

I wasn't talking about aarch64 ILP32, and I suspect neither was Carlos.

In my world, if you still have 32-bit binaries, it's because you lost
the sources or the tools or knowledge to build them at some point.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] nptl: change default stack guard size of threads

Florian Weimer-5
In reply to this post by Wilco Dijkstra-2
On 11/29/2017 11:28 PM, Wilco Dijkstra wrote:
> It's not related to what GLIBC needs, but GLIBC, like Linux, must continue to
> run old binaries so a larger guard size is definitely beneficial. No existing code
> uses probing, so increasing the guard size means far fewer functions could
> jump the guard.  The dropoff is exponential, each doubling of guard page size
> halves the number of functions with a stack larger than the guard size.

That's not actually true in the sense that the math doesn't work out
that way.  If you have a weird function which skips by a really large
amount, you can double the guard size many, many times until the number
of unprotected functions drops further.

And there is definitely a long tail here, due to GNU's affinity to
variable-length arrays and alloca.

> That's why Linux went for a 1MB guard size.

The original intent was to ship only a kernel patch and not change ld.so
at all.  Qualys did some math to support their recommendation, but it
turns out the 1 MiB guard size is insufficient to protect ld.so on some
architectures.  That's why we had to patch ld.so, too.

So far, I haven't seen a strong argument that 64 KiB is better than 32
KiB, or that switching to 96 KiB or 128 KiB would not provide additional
protection.  To me, it's still an arbitrary number.

Based on the ld.so experience, I think it is questionable that existing
vulnerable applications can be fixed by increasing the guard size.  Our
expectation is that we have to recompile with -fstack-clash-protection
to get deterministic crashes (which we are doing with glibc), or to
patch them to avoid the stack jump (which we did for ld.so because the
GCC support wasn't available at the time).

Thanks,
Florian
123