diff mbox series

Thread stack and heap caches - CVE-2019-1010024

Message ID CANUMPcXr+asjC32M1qENUEkBvNj4SGsvE6jNNwsV55H5EhkRiw@mail.gmail.com
State New
Headers show
Series Thread stack and heap caches - CVE-2019-1010024 | expand

Commit Message

Vinay Kumar Nov. 4, 2019, 11:09 a.m. UTC 
Hi,

Regarding bug related to Thread stack and heap caches (CVE-2019-1010024).
https://sourceware.org/bugzilla/show_bug.cgi?id=22852

>> One way to harden is to use a tunable for a thread stack cache, and set that to zero.
Below change in glibc allocatestack.c file gives the expected output
with test case. Verified on x86_64 target.
=======================================================
=======================================================

Output with above patch in glibc trunk:

strace -e mmap,munmap ./thread.out
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
0) = 0x7f387b0df000
mmap(NULL, 139704, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f387b0bc000
mmap(0x7f387b0c3000, 65536, PROT_READ|PROT_EXEC,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x7000) = 0x7f387b0c3000
mmap(0x7f387b0d3000, 20480, PROT_READ,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x17000) = 0x7f387b0d3000
mmap(0x7f387b0d9000, 8192, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1c000) = 0x7f387b0d9000
mmap(0x7f387b0db000, 12728, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f387b0db000
mmap(NULL, 1774424, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f387af0a000
mmap(0x7f387af2f000, 1277952, PROT_READ|PROT_EXEC,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x25000) = 0x7f387af2f000
mmap(0x7f387b067000, 303104, PROT_READ,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x15d000) = 0x7f387b067000
mmap(0x7f387b0b2000, 24576, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1a7000) = 0x7f387b0b2000
mmap(0x7f387b0b8000, 13144, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f387b0b8000
mmap(NULL, 12288, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
0) = 0x7f387af07000
mmap(NULL, 8392704, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_STACK,
-1, 0) = 0x7f387a706000
addr: 0x7f387af03ee0
value deadbeef
malloced 0x7f3874000f70
mmap(NULL, 8392704, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_STACK,
-1, 0) = 0x7f3879f05000
addr: 0x7f387a702ee0
value 0
malloced 0x7f3874000f70
+++ exited with 0 +++

Re-sending due to bounce back of previous mail.

Regards,
Vinay

Comments

Florian Weimer Nov. 4, 2019, 12:25 p.m. UTC | #1 
* Vinay Kumar:

> Regarding bug related to Thread stack and heap caches (CVE-2019-1010024).
> https://sourceware.org/bugzilla/show_bug.cgi?id=22852
>
>>> One way to harden is to use a tunable for a thread stack cache, and set that to zero.
> Below change in glibc allocatestack.c file gives the expected output
> with test case. Verified on x86_64 target.
> =======================================================
> --- a/nptl/allocatestack.c
> +++ b/nptl/allocatestack.c
> @@ -186,6 +186,7 @@ get_cached_stack (size_t *sizep, void **memp)
>        struct pthread *curr;
>
>        curr = list_entry (entry, struct pthread, list);
> +      curr->stackblock_size = 0;
>        if (FREE_P (curr) && curr->stackblock_size >= size)
>         {
>           if (curr->stackblock_size == size)
> =======================================================

Does this really change the randomization?  Won't the kernel map the new
stack at a predictable address, too?

Thanks,
Florian
Vinay Kumar Nov. 11, 2019, 10:56 a.m. UTC | #2 
Hi Florian,

>> Does this really change the randomization?  Won't the kernel map the new
>> stack at a predictable address, too?

Yes, every time new address is assigned as shown in the below
iterations of output.
Also the attached patch "aslr_glibc.patch" adds glibc configuration
option "--with-aslr" to enable randomization.

Compilation command:
$x86_64-windriver-linux-gcc test.c -g -fpie -pie -Xlinker
-rpath=/home/x86_64-tc/prefix-aslr//x86_64-windriver-linux/lib
-Xlinker -I /home
/x86_64-tc/prefix-aslr/x86_64-windriver-linux/lib/ld-2.30.9000.so -w
-lpthread -o thread.out

Output:

Trial 1:
./thread.out
addr: 0x7f15252bfee0
value deadbeef
malloced 0x7f1520000f70
addr: 0x7f1524abeee0
value 0
malloced 0x7f1520000f70

Trial 2:
./thread.out
addr: 0x7f9091640ee0
value deadbeef
malloced 0x7f908c000f70
addr: 0x7f9090e3fee0
value 0
malloced 0x7f908c000f70

Trial 3:
./thread.out
addr: 0x7f0d923dfee0
value deadbeef
malloced 0x7f0d8c000f70
addr: 0x7f0d91bdeee0
value 0
malloced 0x7f0d8c000f70

Trial 4:
./thread.out
addr: 0x7f146d97dee0
value deadbeef
malloced 0x7f1468000f70
addr: 0x7f146d17cee0
value 0
malloced 0x7f1468000f70

Regards,
Vinay
Vinay Kumar Nov. 25, 2019, 4:43 p.m. UTC | #3 
Hi Florian,

Could you please review the last post on randomization.
Bugzilla link : https://sourceware.org/bugzilla/show_bug.cgi?id=22852

Thanks and Regards,
Vinay



On Mon, Nov 11, 2019 at 4:26 PM Vinay Kumar <vinay.m.engg@gmail.com> wrote:
>
> Hi Florian,
>
> >> Does this really change the randomization?  Won't the kernel map the new
> >> stack at a predictable address, too?
>
> Yes, every time new address is assigned as shown in the below
> iterations of output.
> Also the attached patch "aslr_glibc.patch" adds glibc configuration
> option "--with-aslr" to enable randomization.
>
> Compilation command:
> $x86_64-windriver-linux-gcc test.c -g -fpie -pie -Xlinker
> -rpath=/home/x86_64-tc/prefix-aslr//x86_64-windriver-linux/lib
> -Xlinker -I /home
> /x86_64-tc/prefix-aslr/x86_64-windriver-linux/lib/ld-2.30.9000.so -w
> -lpthread -o thread.out
>
> Output:
>
> Trial 1:
> ./thread.out
> addr: 0x7f15252bfee0
> value deadbeef
> malloced 0x7f1520000f70
> addr: 0x7f1524abeee0
> value 0
> malloced 0x7f1520000f70
>
> Trial 2:
> ./thread.out
> addr: 0x7f9091640ee0
> value deadbeef
> malloced 0x7f908c000f70
> addr: 0x7f9090e3fee0
> value 0
> malloced 0x7f908c000f70
>
> Trial 3:
> ./thread.out
> addr: 0x7f0d923dfee0
> value deadbeef
> malloced 0x7f0d8c000f70
> addr: 0x7f0d91bdeee0
> value 0
> malloced 0x7f0d8c000f70
>
> Trial 4:
> ./thread.out
> addr: 0x7f146d97dee0
> value deadbeef
> malloced 0x7f1468000f70
> addr: 0x7f146d17cee0
> value 0
> malloced 0x7f1468000f70
>
> Regards,
> Vinay
Florian Weimer Nov. 25, 2019, 6:10 p.m. UTC | #4 
* Vinay Kumar:

> Hi,
>
> Regarding bug related to Thread stack and heap caches (CVE-2019-1010024).
> https://sourceware.org/bugzilla/show_bug.cgi?id=22852
>
>>> One way to harden is to use a tunable for a thread stack cache, and set that to zero.
> Below change in glibc allocatestack.c file gives the expected output
> with test case. Verified on x86_64 target.
> =======================================================
> --- a/nptl/allocatestack.c
> +++ b/nptl/allocatestack.c
> @@ -186,6 +186,7 @@ get_cached_stack (size_t *sizep, void **memp)
>        struct pthread *curr;
>
>        curr = list_entry (entry, struct pthread, list);
> +      curr->stackblock_size = 0;
>        if (FREE_P (curr) && curr->stackblock_size >= size)
>         {
>           if (curr->stackblock_size == size)
> =======================================================

This will just cause crashes (abort in free_statcks).

I tried to emulate the effect with this program:

#include <err.h>
#include <errno.h>
#include <pthread.h>
#include <stddef.h>
#include <stdint.h>
#include <stdio.h>

static void *
thread (void *closure)
{
  uintptr_t *pp = closure;
  *pp = (uintptr_t) &pp;
  return NULL;
}

int
main (void)
{
  pthread_attr_t attr;
  int ret = pthread_attr_init (&attr);
  if (ret != 0)
    {
      errno = ret;
      err (1, "pthread_attr_init");
    }
  ret = pthread_attr_setstacksize (&attr, 128 * 1024 * 1024);
  if (ret != 0)
    {
      errno = ret;
      err (1, "pthread_attr_setstacksize");
    }

  for (int i = 0; i < 20; ++i)
    {
      pthread_t thr;
      uintptr_t ptr;
      ret = pthread_create (&thr, &attr, thread, &ptr);
      if (ret != 0)
        {
          errno = ret;
          err (1, "pthread_create");
        }
      ret = pthread_join (thr, NULL);
      if (ret != 0)
        {
          errno = ret;
          err (1, "pthread_join");
        }
      printf ("%p\n", (void *) ptr);
    }
}

Its stack size is so large that the stack is never cached.  If you run
it with strace, you will see that mmap and munmap is called for each
iteration.

As I suspected, it prints the same address again and again because the
kernel does NOT randomize mappings.  Until that happens, there is not
much value in disabling the stack cache.

Thanks,
Florian
Vinay Kumar Nov. 25, 2019, 6:29 p.m. UTC | #5 
Hi Florian,

Thanks for the feed back.

FYI,
I had posted the new patch with added glibc configure option
"--with-aslr" which we will not call "get_cached_stack" function.
Also I have posted the output with the testcase from bugzilla.

Link : https://sourceware.org/ml/libc-alpha/2019-11/msg00349.html
Patch link : https://sourceware.org/ml/libc-alpha/2019-11/msg00349/aslr_glibc.patch

Will verify your feedback with my new patch and update you on the same.

Thanks and regards,
Vinay

Regards,
Vinay

On Mon, Nov 25, 2019 at 11:40 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Vinay Kumar:
>
> > Hi,
> >
> > Regarding bug related to Thread stack and heap caches (CVE-2019-1010024).
> > https://sourceware.org/bugzilla/show_bug.cgi?id=22852
> >
> >>> One way to harden is to use a tunable for a thread stack cache, and set that to zero.
> > Below change in glibc allocatestack.c file gives the expected output
> > with test case. Verified on x86_64 target.
> > =======================================================
> > --- a/nptl/allocatestack.c
> > +++ b/nptl/allocatestack.c
> > @@ -186,6 +186,7 @@ get_cached_stack (size_t *sizep, void **memp)
> >        struct pthread *curr;
> >
> >        curr = list_entry (entry, struct pthread, list);
> > +      curr->stackblock_size = 0;
> >        if (FREE_P (curr) && curr->stackblock_size >= size)
> >         {
> >           if (curr->stackblock_size == size)
> > =======================================================
>
> This will just cause crashes (abort in free_statcks).
>
> I tried to emulate the effect with this program:
>
> #include <err.h>
> #include <errno.h>
> #include <pthread.h>
> #include <stddef.h>
> #include <stdint.h>
> #include <stdio.h>
>
> static void *
> thread (void *closure)
> {
>   uintptr_t *pp = closure;
>   *pp = (uintptr_t) &pp;
>   return NULL;
> }
>
> int
> main (void)
> {
>   pthread_attr_t attr;
>   int ret = pthread_attr_init (&attr);
>   if (ret != 0)
>     {
>       errno = ret;
>       err (1, "pthread_attr_init");
>     }
>   ret = pthread_attr_setstacksize (&attr, 128 * 1024 * 1024);
>   if (ret != 0)
>     {
>       errno = ret;
>       err (1, "pthread_attr_setstacksize");
>     }
>
>   for (int i = 0; i < 20; ++i)
>     {
>       pthread_t thr;
>       uintptr_t ptr;
>       ret = pthread_create (&thr, &attr, thread, &ptr);
>       if (ret != 0)
>         {
>           errno = ret;
>           err (1, "pthread_create");
>         }
>       ret = pthread_join (thr, NULL);
>       if (ret != 0)
>         {
>           errno = ret;
>           err (1, "pthread_join");
>         }
>       printf ("%p\n", (void *) ptr);
>     }
> }
>
> Its stack size is so large that the stack is never cached.  If you run
> it with strace, you will see that mmap and munmap is called for each
> iteration.
>
> As I suspected, it prints the same address again and again because the
> kernel does NOT randomize mappings.  Until that happens, there is not
> much value in disabling the stack cache.
>
> Thanks,
> Florian
>
diff mbox series

Patch

--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -186,6 +186,7 @@  get_cached_stack (size_t *sizep, void **memp)
       struct pthread *curr;

       curr = list_entry (entry, struct pthread, list);
+      curr->stackblock_size = 0;
       if (FREE_P (curr) && curr->stackblock_size >= size)
        {
          if (curr->stackblock_size == size)