From patchwork Fri Oct 28 13:17:22 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 79925 Delivered-To: patch@linaro.org Received: by 10.140.97.247 with SMTP id m110csp1172645qge; Fri, 28 Oct 2016 06:17:50 -0700 (PDT) X-Received: by 10.98.223.150 with SMTP id d22mr24990468pfl.2.1477660670085; Fri, 28 Oct 2016 06:17:50 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id m5si13604369pgk.124.2016.10.28.06.17.49 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 28 Oct 2016 06:17:50 -0700 (PDT) Received-SPF: pass (google.com: domain of libc-alpha-return-74171-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org; spf=pass (google.com: domain of libc-alpha-return-74171-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=libc-alpha-return-74171-patch=linaro.org@sourceware.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:from:subject:message-id:date:mime-version :content-type; q=dns; s=default; b=drKL9uSuLXejPWGh7LVWxS4YsMPtz w5yR9oVDM3RfKW99XwrA55QEIshOMNPl4lBXIp5qqhw/Li48TvGdz5pYGdfGuERz qf1dziUTArP/couJxxp1+LzNvrDD0PbPVayFkNK5u1x9QRxw5PdBOTDoA0OOKnFc Vd5rXWSbAUdDe4= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:from:subject:message-id:date:mime-version :content-type; s=default; bh=m1WQMeZdHCeK5tdlIeRCx/O/gB4=; b=Jnp f18v2sGZHIlIHTH2iSLNA0mdghOPjDn2PDh2/vyjTg98n8Nl79KhQ+KZNnRZwwdT IU+Wj1QIybyBwjwMxzZF1XQ5CVBpaHMeFSW4+ULfNEwtjcSQT+JkMUTPgZXbm3Sb D+rKIlYbl63zh7esVKRf5muZGlmZlxIw8tIH//JA= Received: (qmail 72171 invoked by alias); 28 Oct 2016 13:17:37 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 72154 invoked by uid 89); 28 Oct 2016 13:17:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Apply, lie, 42, 6, 30362 X-HELO: mx1.redhat.com To: GNU C Library From: Florian Weimer Subject: [PATCH] malloc: Implement heap protector Message-ID: Date: Fri, 28 Oct 2016 15:17:22 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 This is a fully working version of a heap protector, which uses XOR cookies to obfuscate the heap metadata. The hope is that this makes exploitation of simple heap overflows more difficult because the attackers have to obtain the heap guard values first before they can create a malloc chunk that is recognized by the malloc implementation. I verified that existing Emacs binaries which contain a dumped heap will still work after this change. I still need to redo the performance analysis. An older version of the code had these results for one of DJ's workload files: Welch Two Sample t-test data: old_malloc and new_malloc t = -5.0042, df = 157.65, p-value = 1.484e-06 alternative hypothesis: true difference in means is not equal to 0 95 percent confidence interval: -4.267772 -1.852228 sample estimates: mean of x mean of y 131.07 134.13 Welch Two Sample t-test data: old_calloc and new_calloc t = -0.90822, df = 197.05, p-value = 0.3649 alternative hypothesis: true difference in means is not equal to 0 95 percent confidence interval: -8.435823 3.115823 sample estimates: mean of x mean of y 206.83 209.49 Welch Two Sample t-test data: old_realloc and new_realloc t = -4.7164, df = 122.86, p-value = 6.406e-06 alternative hypothesis: true difference in means is not equal to 0 95 percent confidence interval: -4.202311 -1.717689 sample estimates: mean of x mean of y 139.70 142.66 Welch Two Sample t-test data: old_free and new_free t = -3.0362, df = 105.61, p-value = 0.003018 alternative hypothesis: true difference in means is not equal to 0 95 percent confidence interval: -1.4546563 -0.3053437 sample estimates: mean of x mean of y 96.47 97.35 So 3 cycles for malloc, realloc and probably calloc, and one cycle for free. I still hope to recover some of the performance loss with micro-optimizations, but I'd like to get the patch committed ASAP to increase testing time before the release. Florian malloc: Implement heap protector 2016-10-27 Florian Weimer * malloc/mallc-internal.h (__malloc_header_guard) (__malloc_footer_guard): Declare. * malloc/malloc-guard.c: New file. * malloc/Makefile (routines): Add it. * malloc/malloc.c (HEAP_CRYPT_SIZE, HEAP_CRYPT_PREVSIZE): Define. (chunksize_nomask, prev_size, set_prev_size, set_head_size) (set_head, set_foot): Add encryption. * malloc/arena.c (ptmalloc_init): For shared builds, initialize the heap guard variables. Initialize the top chunk. * malloc/hooks.c (malloc_set_state): Apply the heap guard to the dumped heap. * malloc/tst-mallocstate.c (malloc_usable_size_valid): New variable. (check_allocation): Check malloc_usable_size result if malloc_usable_size_valid. (init_heap): Set malloc_usable_size_valid. * csu/libc-start.c (LIBC_START_MAIN): Initialize heap guard variables. * sysdeps/generic/ldsodefs.h (struct rtld_global_ro): Add members _dl_malloc_header_guard, _dl_malloc_footer_guard. * elf/rtld.c (security_init): Initialize temporary copy of the heap guard variables. diff --git a/csu/libc-start.c b/csu/libc-start.c index 333a4cc..ae6abde 100644 --- a/csu/libc-start.c +++ b/csu/libc-start.c @@ -22,6 +22,7 @@ #include #include #include +#include extern void __libc_init_first (int argc, char **argv, char **envp); @@ -210,6 +211,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), __pointer_chk_guard_local = keys.pointer; # endif + /* In the non-shared case, we initialize the heap guard + directly. */ + __malloc_header_guard = keys.heap_header; + __malloc_footer_guard = keys.heap_footer; + #endif /* Register the destructor of the dynamic linker if there is any. */ diff --git a/elf/rtld.c b/elf/rtld.c index de965da..7ea06e4 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -42,6 +42,7 @@ #include #include #include +#include #include @@ -716,6 +717,11 @@ security_init (void) #endif __pointer_chk_guard_local = keys.pointer; + /* Keep a copy of the computed keys, so that they can be obtained + during malloc initialization in libc.so. */ + GLRO (dl_malloc_header_guard) = keys.heap_header; + GLRO (dl_malloc_footer_guard) = keys.heap_footer; + /* We do not need the _dl_random value anymore. The less information we leave behind, the better, so clear the variable. */ diff --git a/malloc/Makefile b/malloc/Makefile index b8efcd6..cd289f8 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -41,7 +41,7 @@ tests-static := \ tests += $(tests-static) test-srcs = tst-mtrace -routines = malloc morecore mcheck mtrace obstack \ +routines = malloc morecore mcheck mtrace obstack malloc-guard \ scratch_buffer_grow scratch_buffer_grow_preserve \ scratch_buffer_set_array_size diff --git a/malloc/arena.c b/malloc/arena.c index f85b0af..792451a 100644 --- a/malloc/arena.c +++ b/malloc/arena.c @@ -340,6 +340,19 @@ ptmalloc_init (void) if (check_action != 0) __malloc_check_init (); } + +#ifdef SHARED + /* For a shared library, elf/rtld.c performed key setup in + security_init, and we copy the keys. In static builds, the guard + cookies have already been initialized in csu/libc-start.c. */ + __malloc_header_guard = GLRO (dl_malloc_header_guard); + __malloc_footer_guard = GLRO (dl_malloc_footer_guard); +#endif + + /* Initialize the top chunk, based on the heap protector guards. */ + malloc_init_state (&main_arena); + set_head (main_arena.top, 0); + #if HAVE_MALLOC_INIT_HOOK void (*hook) (void) = atomic_forced_read (__malloc_initialize_hook); if (hook != NULL) diff --git a/malloc/hooks.c b/malloc/hooks.c index 12995d3..8bb2b74 100644 --- a/malloc/hooks.c +++ b/malloc/hooks.c @@ -537,35 +537,38 @@ malloc_set_state (void *msptr) dumped_main_arena_end, realloc and free will recognize these chunks as dumped fake mmapped chunks and never free them. */ - /* Find the chunk with the lowest address with the heap. */ - mchunkptr chunk = NULL; + /* Find the chunk with the lowest address with the heap. If + successful, size_header will point to the mchunk_size member (not + the chunk start, i.e. the mchunck_prev_size member). */ + size_t *size_header = NULL; { size_t *candidate = (size_t *) ms->sbrk_base; size_t *end = (size_t *) (ms->sbrk_base + ms->sbrked_mem_bytes); while (candidate < end) if (*candidate != 0) { - chunk = mem2chunk ((void *) (candidate + 1)); + size_header = candidate; break; } else ++candidate; } - if (chunk == NULL) + if (size_header == NULL) return 0; /* Iterate over the dumped heap and patch the chunks so that they - are treated as fake mmapped chunks. */ + are treated as fake mmapped chunks. We cannot use the regular + accessors because the chunks we read are not yet encrypted. */ mchunkptr top = ms->av[2]; - while (chunk < top) + size_t *top_size_header = ((size_t *) top) + 1; + while (size_header < top_size_header) { - if (inuse (chunk)) - { - /* Mark chunk as mmapped, to trigger the fallback path. */ - size_t size = chunksize (chunk); - set_head (chunk, size | IS_MMAPPED); - } - chunk = next_chunk (chunk); + size_t size = *size_header & ~SIZE_BITS; + /* We treat all chunks as allocated. The heap consistency + checks do not trigger because they are not active for the + dumped heap. */ + *size_header = HEAP_CRYPT_SIZE (size) | IS_MMAPPED; + size_header += size / sizeof (*size_header); } /* The dumped fake mmapped chunks all lie in this address range. */ diff --git a/malloc/malloc-guard.c b/malloc/malloc-guard.c new file mode 100644 index 0000000..c8b3581 --- /dev/null +++ b/malloc/malloc-guard.c @@ -0,0 +1,29 @@ +/* Heap protector variables. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public License as + published by the Free Software Foundation; either version 2.1 of the + License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; see the file COPYING.LIB. If + not, see . */ + +/* These variables are defined in a separate file because the static + startup code initializes them, but this should not pull the rest of + the libc malloc implementation into the link. */ + +#include + +/* The heap cookie. The lowest three bits (corresponding to + SIZE_BITS) in __malloc_guard_header must be clear. Initialized + during libc startup, and computed by elf/dl-keysetup.c. */ +INTERNAL_SIZE_T __malloc_header_guard; /* For size. */ +INTERNAL_SIZE_T __malloc_footer_guard; /* For prev_size. */ diff --git a/malloc/malloc-internal.h b/malloc/malloc-internal.h index a3df8c3..e723539 100644 --- a/malloc/malloc-internal.h +++ b/malloc/malloc-internal.h @@ -81,5 +81,8 @@ void __malloc_fork_unlock_parent (void) internal_function attribute_hidden; /* Called in the child process after a fork. */ void __malloc_fork_unlock_child (void) internal_function attribute_hidden; +/* Random values for the heap protector. */ +extern INTERNAL_SIZE_T __malloc_header_guard attribute_hidden; +extern INTERNAL_SIZE_T __malloc_footer_guard attribute_hidden; #endif /* _MALLOC_INTERNAL_H */ diff --git a/malloc/malloc.c b/malloc/malloc.c index 72d22bd..e1e732d 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -1221,6 +1221,10 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ /* Mark a chunk as not being on the main arena. */ #define set_non_main_arena(p) ((p)->mchunk_size |= NON_MAIN_ARENA) +/* Decrypt a heap header chunk. */ +#define HEAP_CRYPT_SIZE(val) (__malloc_header_guard ^ ((INTERNAL_SIZE_T) val)) +#define HEAP_CRYPT_PREVSIZE(val) \ + (__malloc_footer_guard ^ ((INTERNAL_SIZE_T) val)) /* Bits to mask off when extracting size @@ -1236,16 +1240,16 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ #define chunksize(p) (chunksize_nomask (p) & ~(SIZE_BITS)) /* Like chunksize, but do not mask SIZE_BITS. */ -#define chunksize_nomask(p) ((p)->mchunk_size) +#define chunksize_nomask(p) HEAP_CRYPT_SIZE ((p)->mchunk_size) /* Ptr to next physical malloc_chunk. */ #define next_chunk(p) ((mchunkptr) (((char *) (p)) + chunksize (p))) /* Size of the chunk below P. Only valid if prev_inuse (P). */ -#define prev_size(p) ((p)->mchunk_prev_size) +#define prev_size(p) HEAP_CRYPT_PREVSIZE ((p)->mchunk_prev_size) /* Set the size of the chunk below P. Only valid if prev_inuse (P). */ -#define set_prev_size(p, sz) ((p)->mchunk_prev_size = (sz)) +#define set_prev_size(p, sz) ((p)->mchunk_prev_size = HEAP_CRYPT_PREVSIZE (sz)) /* Ptr to previous physical malloc_chunk. Only valid if prev_inuse (P). */ #define prev_chunk(p) ((mchunkptr) (((char *) (p)) - prev_size (p))) @@ -1277,13 +1281,16 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ /* Set size at head, without disturbing its use bit */ -#define set_head_size(p, s) ((p)->mchunk_size = (((p)->mchunk_size & SIZE_BITS) | (s))) +#define set_head_size(p, s) \ + ((p)->mchunk_size = ((p)->mchunk_size & SIZE_BITS) | HEAP_CRYPT_SIZE (s)) /* Set size/use field */ -#define set_head(p, s) ((p)->mchunk_size = (s)) +#define set_head(p, s) ((p)->mchunk_size = HEAP_CRYPT_SIZE (s)) /* Set size at footer (only when chunk is not in use) */ -#define set_foot(p, s) (((mchunkptr) ((char *) (p) + (s)))->mchunk_prev_size = (s)) +#define set_foot(p, s) \ + (((mchunkptr) ((char *) (p) + (s)))->mchunk_prev_size \ + = HEAP_CRYPT_PREVSIZE (s)) #pragma GCC poison mchunk_size diff --git a/malloc/tst-mallocstate.c b/malloc/tst-mallocstate.c index 7e081c5..bd6678e 100644 --- a/malloc/tst-mallocstate.c +++ b/malloc/tst-mallocstate.c @@ -186,6 +186,10 @@ struct allocation unsigned int seed; }; +/* After heap initialization, we can call malloc_usable_size to check + if it gives valid results. */ +static bool malloc_usable_size_valid; + /* Check that the allocation task allocation has the expected contents. */ static void @@ -221,6 +225,23 @@ check_allocation (const struct allocation *alloc, int index) putc ('\n', stdout); errors = true; } + + if (malloc_usable_size_valid) + { + size_t usable = malloc_usable_size (alloc->data); + if (usable < size) + { + printf ("error: allocation %d has reported size %zu (expected %zu)\n", + index, usable, size); + errors = true; + } + else if (usable > size + 4096) + { + printf ("error: allocation %d reported as %zu bytes (requested %zu)\n", + index, usable, size); + errors = true; + } + } } /* A heap allocation combined with pending actions on it. */ @@ -317,6 +338,10 @@ init_heap (void) write_message ("error: malloc_set_state failed\n"); _exit (1); } + + /* The heap has been initialized. We may now call + malloc_usable_size. */ + malloc_usable_size_valid = true; } /* Interpose the initialization callback. */ diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h index f68fdf4..801ded8 100644 --- a/sysdeps/generic/ldsodefs.h +++ b/sysdeps/generic/ldsodefs.h @@ -607,6 +607,10 @@ struct rtld_global_ro /* List of auditing interfaces. */ struct audit_ifaces *_dl_audit; unsigned int _dl_naudit; + + /* malloc protection keys. */ + uintptr_t _dl_malloc_header_guard; + uintptr_t _dl_malloc_footer_guard; }; # define __rtld_global_attribute__ # if IS_IN (rtld)