Message ID | 5A32A3D6.5010200@arm.com |
---|---|
State | New |
Headers | show |
Series | [RFC,v2] aarch64: enforce >=64K guard size | expand |
On 14/12/17 16:16, Szabolcs Nagy wrote: > v2: > - only change guard size on aarch64 > - don't report the inflated guard size > - this is on top of > https://sourceware.org/ml/libc-alpha/2017-12/msg00368.html > > There are several compiler implementations that allow large stack > allocations to jump over the guard page at the end of the stack and > corrupt memory beyond that. See CVE-2017-1000364. > > Compilers can emit code to probe the stack such that the guard page > cannot be skipped, but on aarch64 the probe interval is 64K instead > of the minimum supported page size (4K). > > This patch enforces at least 64K guard on aarch64 unless the guard > is disabled by setting its size to 0. For backward compatibility > reasons the increased guard is not reported, so it is only observable > by exhausting the address space or parsing /proc/self/maps on linux. > > The patch does not affect threads with user allocated stacks. > > 2017-12-14 Szabolcs Nagy <szabolcs.nagy@arm.com> > > * nptl/allocatestack.c (allocate_stack): Use ARCH_MIN_GUARD_SIZE. > * nptl/descr.h (ARCH_MIN_GUARD_SIZE): Define. > * sysdeps/aarch64/nptl/pthreaddef.h (ARCH_MIN_GUARD_SIZE): Define. > ping.
On 12/14/2017 08:16 AM, Szabolcs Nagy wrote: > v2: > - only change guard size on aarch64 > - don't report the inflated guard size > - this is on top of > https://sourceware.org/ml/libc-alpha/2017-12/msg00368.html > > There are several compiler implementations that allow large stack > allocations to jump over the guard page at the end of the stack and > corrupt memory beyond that. See CVE-2017-1000364. > > Compilers can emit code to probe the stack such that the guard page > cannot be skipped, but on aarch64 the probe interval is 64K instead > of the minimum supported page size (4K). > > This patch enforces at least 64K guard on aarch64 unless the guard > is disabled by setting its size to 0. For backward compatibility > reasons the increased guard is not reported, so it is only observable > by exhausting the address space or parsing /proc/self/maps on linux. > > The patch does not affect threads with user allocated stacks. > > 2017-12-14 Szabolcs Nagy <szabolcs.nagy@arm.com> > > * nptl/allocatestack.c (allocate_stack): Use ARCH_MIN_GUARD_SIZE. > * nptl/descr.h (ARCH_MIN_GUARD_SIZE): Define. > * sysdeps/aarch64/nptl/pthreaddef.h (ARCH_MIN_GUARD_SIZE): Define. OK if you expand the comment and fix the macro api. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c > index 9525322b1f92bb34aa21dcab28566aecd7434e90..9d47b86cbfc45e40c06e0ee13889fcce48902261 100644 > --- a/nptl/allocatestack.c > +++ b/nptl/allocatestack.c > @@ -520,6 +520,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, > { > /* Allocate some anonymous memory. If possible use the cache. */ > size_t guardsize; > + size_t reported_guardsize; > size_t reqsize; > void *mem; > const int prot = (PROT_READ | PROT_WRITE > @@ -530,8 +531,14 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, > assert (size != 0); > > /* Make sure the size of the stack is enough for the guard and > - eventually the thread descriptor. */ > + eventually the thread descriptor. On some targets there is > + a minimum guard size requirement, ARCH_MIN_GUARD_SIZE, so > + internally enforce it (unless the guard was disabled), but > + report the original guard size for backward compatibility. */ Please document, in the comment, the exact backwards compatibility case we are solving here. > guardsize = (attr->guardsize + pagesize_m1) & ~pagesize_m1; > + reported_guardsize = guardsize; > + if (guardsize > 0 && guardsize < ARCH_MIN_GUARD_SIZE) > + guardsize = ARCH_MIN_GUARD_SIZE; > size += guardsize; > if (__builtin_expect (size < ((guardsize + __static_tls_size > + MINIMAL_REST_STACK + pagesize_m1) > @@ -740,7 +747,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, > /* The pthread_getattr_np() calls need to get passed the size > requested in the attribute, regardless of how large the > actually used guardsize is. */ > - pd->reported_guardsize = guardsize; > + pd->reported_guardsize = reported_guardsize; > } > > /* Initialize the lock. We have to do this unconditionally since the > diff --git a/nptl/descr.h b/nptl/descr.h > index c83b17b674b07e5b4e2cbaecf984f6d1673187b5..b5f9412eec0a09e8af10eb9e0c5ff2a8b083559d 100644 > --- a/nptl/descr.h > +++ b/nptl/descr.h > @@ -39,6 +39,10 @@ > # define TCB_ALIGNMENT sizeof (double) > #endif > > +#ifndef ARCH_MIN_GUARD_SIZE > +# define ARCH_MIN_GUARD_SIZE 0 > +#endif This is the "centralized defaults" pattern. This is not a typo-proof macro API. All machines must define ARCH_MIN_GUARD_SIZE to zero in their pthreaddef.h instead of in ntpl/descr.h. See: https://sourceware.org/glibc/wiki/Wundef > + > > /* We keep thread specific data in a special data structure, a two-level > array. The top-level array contains pointers to dynamically allocated > diff --git a/sysdeps/aarch64/nptl/pthreaddef.h b/sysdeps/aarch64/nptl/pthreaddef.h > index d0411a57a1f1356d7e3961f65b733a6b6eb96ae1..5d4c90f83f2b3f3759ab15ee2bc818078ec1f150 100644 > --- a/sysdeps/aarch64/nptl/pthreaddef.h > +++ b/sysdeps/aarch64/nptl/pthreaddef.h > @@ -19,6 +19,9 @@ > /* Default stack size. */ > #define ARCH_STACK_DEFAULT_SIZE (2 * 1024 * 1024) > > +/* Minimum guard size. */ > +#define ARCH_MIN_GUARD_SIZE (64 * 1024) > + > /* Required stack pointer alignment at beginning. */ > #define STACK_ALIGN 16 >
diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index 9525322b1f92bb34aa21dcab28566aecd7434e90..9d47b86cbfc45e40c06e0ee13889fcce48902261 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -520,6 +520,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, { /* Allocate some anonymous memory. If possible use the cache. */ size_t guardsize; + size_t reported_guardsize; size_t reqsize; void *mem; const int prot = (PROT_READ | PROT_WRITE @@ -530,8 +531,14 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, assert (size != 0); /* Make sure the size of the stack is enough for the guard and - eventually the thread descriptor. */ + eventually the thread descriptor. On some targets there is + a minimum guard size requirement, ARCH_MIN_GUARD_SIZE, so + internally enforce it (unless the guard was disabled), but + report the original guard size for backward compatibility. */ guardsize = (attr->guardsize + pagesize_m1) & ~pagesize_m1; + reported_guardsize = guardsize; + if (guardsize > 0 && guardsize < ARCH_MIN_GUARD_SIZE) + guardsize = ARCH_MIN_GUARD_SIZE; size += guardsize; if (__builtin_expect (size < ((guardsize + __static_tls_size + MINIMAL_REST_STACK + pagesize_m1) @@ -740,7 +747,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, /* The pthread_getattr_np() calls need to get passed the size requested in the attribute, regardless of how large the actually used guardsize is. */ - pd->reported_guardsize = guardsize; + pd->reported_guardsize = reported_guardsize; } /* Initialize the lock. We have to do this unconditionally since the diff --git a/nptl/descr.h b/nptl/descr.h index c83b17b674b07e5b4e2cbaecf984f6d1673187b5..b5f9412eec0a09e8af10eb9e0c5ff2a8b083559d 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -39,6 +39,10 @@ # define TCB_ALIGNMENT sizeof (double) #endif +#ifndef ARCH_MIN_GUARD_SIZE +# define ARCH_MIN_GUARD_SIZE 0 +#endif + /* We keep thread specific data in a special data structure, a two-level array. The top-level array contains pointers to dynamically allocated diff --git a/sysdeps/aarch64/nptl/pthreaddef.h b/sysdeps/aarch64/nptl/pthreaddef.h index d0411a57a1f1356d7e3961f65b733a6b6eb96ae1..5d4c90f83f2b3f3759ab15ee2bc818078ec1f150 100644 --- a/sysdeps/aarch64/nptl/pthreaddef.h +++ b/sysdeps/aarch64/nptl/pthreaddef.h @@ -19,6 +19,9 @@ /* Default stack size. */ #define ARCH_STACK_DEFAULT_SIZE (2 * 1024 * 1024) +/* Minimum guard size. */ +#define ARCH_MIN_GUARD_SIZE (64 * 1024) + /* Required stack pointer alignment at beginning. */ #define STACK_ALIGN 16