From 08dd8655ae276d12d1ea9f075871ecf6ad69d229 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 6 Dec 2017 22:29:33 -0800 Subject: [PATCH 3/3] Narrow pointer bounds when appropriate This typically occurs in a storage manager, where the caller is expected to access only the newly-allocated object, instead of using the returned value to access unrelated parts of the heap. * src/alloc.c (allocate_string, allocate_string_data) (compact_small_strings, find_string_data_in_pure) (sweep_strings, setup_on_free_list, allocate_vectorlike (pure_alloc): * src/bytecode.c (exec_byte_code): * src/callint.c (Fcall_interactively): * src/dispnew.c (scrolling): * src/editfns.c (styled_format): * src/frame.c (xrdb_get_resource, x_get_resource_string): * src/fringe.c (Fdefine_fringe_bitmap): * src/gmalloc.c (malloc, realloc, aligned_alloc): Narrow pointer bounds when appropriate. * src/alloc.c (SDATA_OF_STRING): * src/lisp.h (make_lisp_symbol) [__CHKP__]: Widen bounds here, though. * src/bytecode.c, src/callint.c, src/dispnew.c, src/editfns.c: * src/emacs.c, src/frame.c, src/fringe.c: Include ptr-bounds.h. * src/ptr-bounds.h (ptr_bounds_clip): New function. --- src/alloc.c | 31 +++++++++++++++++-------------- src/bytecode.c | 15 +++++++++------ src/callint.c | 4 ++++ src/dispnew.c | 6 ++++++ src/editfns.c | 9 ++++++--- src/emacs.c | 1 + src/frame.c | 5 +++++ src/fringe.c | 5 ++++- src/gmalloc.c | 10 +++++++--- src/lisp.h | 9 +++++++-- src/ptr-bounds.h | 27 +++++++++++++++++++++++++++ 11 files changed, 93 insertions(+), 29 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index 96b9aaa0d2..9197ff12ef 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -1727,7 +1727,8 @@ static EMACS_INT total_string_bytes; a pointer to the `u.data' member of its sdata structure; the structure starts at a constant offset in front of that. */ -#define SDATA_OF_STRING(S) ((sdata *) ((S)->u.s.data - SDATA_DATA_OFFSET)) +#define SDATA_OF_STRING(S) ptr_bounds_init ((sdata *) ((S)->u.s.data \ + - SDATA_DATA_OFFSET)) #ifdef GC_CHECK_STRING_OVERRUN @@ -1919,7 +1920,7 @@ allocate_string (void) /* Every string on a free list should have NULL data pointer. */ s->u.s.data = NULL; NEXT_FREE_LISP_STRING (s) = string_free_list; - string_free_list = s; + string_free_list = ptr_bounds_clip (s, sizeof *s); } total_free_strings += STRING_BLOCK_SIZE; @@ -2034,7 +2035,7 @@ allocate_string_data (struct Lisp_String *s, MALLOC_UNBLOCK_INPUT; - s->u.s.data = SDATA_DATA (data); + s->u.s.data = ptr_bounds_clip (SDATA_DATA (data), nbytes + 1); #ifdef GC_CHECK_STRING_BYTES SDATA_NBYTES (data) = nbytes; #endif @@ -2120,7 +2121,7 @@ sweep_strings (void) /* Put the string on the free-list. */ NEXT_FREE_LISP_STRING (s) = string_free_list; - string_free_list = s; + string_free_list = ptr_bounds_clip (s, sizeof *s); ++nfree; } } @@ -2128,7 +2129,7 @@ sweep_strings (void) { /* S was on the free-list before. Put it there again. */ NEXT_FREE_LISP_STRING (s) = string_free_list; - string_free_list = s; + string_free_list = ptr_bounds_clip (s, sizeof *s); ++nfree; } } @@ -2224,9 +2225,9 @@ compact_small_strings (void) nbytes = s ? STRING_BYTES (s) : SDATA_NBYTES (from); eassert (nbytes <= LARGE_STRING_BYTES); - nbytes = SDATA_SIZE (nbytes); + ptrdiff_t size = SDATA_SIZE (nbytes); sdata *from_end = (sdata *) ((char *) from - + nbytes + GC_STRING_EXTRA); + + size + GC_STRING_EXTRA); #ifdef GC_CHECK_STRING_OVERRUN if (memcmp (string_overrun_cookie, @@ -2240,22 +2241,23 @@ compact_small_strings (void) { /* If TB is full, proceed with the next sblock. */ sdata *to_end = (sdata *) ((char *) to - + nbytes + GC_STRING_EXTRA); + + size + GC_STRING_EXTRA); if (to_end > tb_end) { tb->next_free = to; tb = tb->next; tb_end = (sdata *) ((char *) tb + SBLOCK_SIZE); to = tb->data; - to_end = (sdata *) ((char *) to + nbytes + GC_STRING_EXTRA); + to_end = (sdata *) ((char *) to + size + GC_STRING_EXTRA); } /* Copy, and update the string's `data' pointer. */ if (from != to) { eassert (tb != b || to < from); - memmove (to, from, nbytes + GC_STRING_EXTRA); - to->string->u.s.data = SDATA_DATA (to); + memmove (to, from, size + GC_STRING_EXTRA); + to->string->u.s.data + = ptr_bounds_clip (SDATA_DATA (to), nbytes + 1); } /* Advance past the sdata we copied to. */ @@ -3038,6 +3040,7 @@ static EMACS_INT total_vector_slots, total_free_vector_slots; static void setup_on_free_list (struct Lisp_Vector *v, ptrdiff_t nbytes) { + v = ptr_bounds_clip (v, nbytes); eassume (header_size <= nbytes); ptrdiff_t nwords = (nbytes - header_size) / word_size; XSETPVECTYPESIZE (v, PVEC_FREE, 0, nwords); @@ -3347,7 +3350,7 @@ allocate_vectorlike (ptrdiff_t len) MALLOC_UNBLOCK_INPUT; - return p; + return ptr_bounds_clip (p, nbytes); } } @@ -5358,7 +5361,7 @@ pure_alloc (size_t size, int type) pure_bytes_used = pure_bytes_used_lisp + pure_bytes_used_non_lisp; if (pure_bytes_used <= pure_size) - return result; + return ptr_bounds_clip (result, size); /* Don't allocate a large amount here, because it might get mmap'd and then its address @@ -5443,7 +5446,7 @@ find_string_data_in_pure (const char *data, ptrdiff_t nbytes) /* Check the remaining characters. */ if (memcmp (data, non_lisp_beg + start, nbytes) == 0) /* Found. */ - return non_lisp_beg + start; + return ptr_bounds_clip (non_lisp_beg + start, nbytes + 1); start += last_char_skip; } diff --git a/src/bytecode.c b/src/bytecode.c index 8746568f16..78207f776c 100644 --- a/src/bytecode.c +++ b/src/bytecode.c @@ -24,6 +24,7 @@ along with GNU Emacs. If not, see . */ #include "character.h" #include "buffer.h" #include "keyboard.h" +#include "ptr-bounds.h" #include "syntax.h" #include "window.h" @@ -363,13 +364,15 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, Lisp_Object maxdepth, unsigned char quitcounter = 1; EMACS_INT stack_items = XFASTINT (maxdepth) + 1; USE_SAFE_ALLOCA; - Lisp_Object *stack_base; - SAFE_ALLOCA_LISP_EXTRA (stack_base, stack_items, bytestr_length); - Lisp_Object *stack_lim = stack_base + stack_items; + void *alloc; + SAFE_ALLOCA_LISP_EXTRA (alloc, stack_items, bytestr_length); + ptrdiff_t item_bytes = stack_items * word_size; + Lisp_Object *stack_base = ptr_bounds_clip (alloc, item_bytes); Lisp_Object *top = stack_base; - memcpy (stack_lim, SDATA (bytestr), bytestr_length); - void *void_stack_lim = stack_lim; - unsigned char const *bytestr_data = void_stack_lim; + Lisp_Object *stack_lim = stack_base + stack_items; + unsigned char *bytestr_data = alloc; + bytestr_data = ptr_bounds_clip (bytestr_data + item_bytes, bytestr_length); + memcpy (bytestr_data, SDATA (bytestr), bytestr_length); unsigned char const *pc = bytestr_data; ptrdiff_t count = SPECPDL_INDEX (); diff --git a/src/callint.c b/src/callint.c index 5d88082e38..df26e4abb5 100644 --- a/src/callint.c +++ b/src/callint.c @@ -21,6 +21,7 @@ along with GNU Emacs. If not, see . */ #include #include "lisp.h" +#include "ptr-bounds.h" #include "character.h" #include "buffer.h" #include "keyboard.h" @@ -494,6 +495,9 @@ invoke it. If KEYS is omitted or nil, the return value of varies = (signed char *) (visargs + nargs); memclear (args, nargs * (2 * word_size + 1)); + args = ptr_bounds_clip (args, nargs * sizeof *args); + visargs = ptr_bounds_clip (visargs, nargs * sizeof *visargs); + varies = ptr_bounds_clip (varies, nargs * sizeof *varies); if (!NILP (enable)) specbind (Qenable_recursive_minibuffers, Qt); diff --git a/src/dispnew.c b/src/dispnew.c index b0fc5c31fa..7fea867f66 100644 --- a/src/dispnew.c +++ b/src/dispnew.c @@ -25,6 +25,7 @@ along with GNU Emacs. If not, see . */ #include #include "lisp.h" +#include "ptr-bounds.h" #include "termchar.h" /* cm.h must come after dispextern.h on Windows. */ #include "dispextern.h" @@ -4652,6 +4653,11 @@ scrolling (struct frame *frame) unsigned *new_hash = old_hash + height; int *draw_cost = (int *) (new_hash + height); int *old_draw_cost = draw_cost + height; + old_hash = ptr_bounds_clip (old_hash, height * sizeof *old_hash); + new_hash = ptr_bounds_clip (new_hash, height * sizeof *new_hash); + draw_cost = ptr_bounds_clip (draw_cost, height * sizeof *draw_cost); + old_draw_cost = ptr_bounds_clip (old_draw_cost, + height * sizeof *old_draw_cost); eassert (current_matrix); diff --git a/src/editfns.c b/src/editfns.c index e671ba0761..a333dc0801 100644 --- a/src/editfns.c +++ b/src/editfns.c @@ -56,6 +56,7 @@ along with GNU Emacs. If not, see . */ #include "composite.h" #include "intervals.h" +#include "ptr-bounds.h" #include "character.h" #include "buffer.h" #include "coding.h" @@ -4208,9 +4209,9 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message) ptrdiff_t nspec_bound = SCHARS (args[0]) >> 1; /* Allocate the info and discarded tables. */ - ptrdiff_t alloca_size; - if (INT_MULTIPLY_WRAPV (nspec_bound, sizeof *info, &alloca_size) - || INT_ADD_WRAPV (formatlen, alloca_size, &alloca_size) + ptrdiff_t info_size, alloca_size; + if (INT_MULTIPLY_WRAPV (nspec_bound, sizeof *info, &info_size) + || INT_ADD_WRAPV (formatlen, info_size, &alloca_size) || SIZE_MAX < alloca_size) memory_full (SIZE_MAX); info = SAFE_ALLOCA (alloca_size); @@ -4218,6 +4219,8 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message) string was not copied into the output. It is 2 if byte I was not the first byte of its character. */ char *discarded = (char *) &info[nspec_bound]; + info = ptr_bounds_clip (info, info_size); + discarded = ptr_bounds_clip (discarded, formatlen); memset (discarded, 0, formatlen); /* Try to determine whether the result should be multibyte. diff --git a/src/emacs.c b/src/emacs.c index 808abcd9aa..06dbe92f99 100644 --- a/src/emacs.c +++ b/src/emacs.c @@ -83,6 +83,7 @@ along with GNU Emacs. If not, see . */ #include "charset.h" #include "composite.h" #include "dispextern.h" +#include "ptr-bounds.h" #include "regex.h" #include "sheap.h" #include "syntax.h" diff --git a/src/frame.c b/src/frame.c index 5bafbeddcc..94ec9fbdc7 100644 --- a/src/frame.c +++ b/src/frame.c @@ -35,6 +35,7 @@ along with GNU Emacs. If not, see . */ #include "buffer.h" /* These help us bind and responding to switch-frame events. */ #include "keyboard.h" +#include "ptr-bounds.h" #include "frame.h" #include "blockinput.h" #include "termchar.h" @@ -4812,6 +4813,8 @@ xrdb_get_resource (XrmDatabase rdb, Lisp_Object attribute, Lisp_Object class, Li USE_SAFE_ALLOCA; char *name_key = SAFE_ALLOCA (name_keysize + class_keysize); char *class_key = name_key + name_keysize; + name_key = ptr_bounds_clip (name_key, name_keysize); + class_key = ptr_bounds_clip (class_key, class_keysize); /* Start with emacs.FRAMENAME for the name (the specific one) and with `Emacs' for the class key (the general one). */ @@ -4890,6 +4893,8 @@ x_get_resource_string (const char *attribute, const char *class) ptrdiff_t class_keysize = sizeof (EMACS_CLASS) - 1 + strlen (class) + 2; char *name_key = SAFE_ALLOCA (name_keysize + class_keysize); char *class_key = name_key + name_keysize; + name_key = ptr_bounds_clip (name_key, name_keysize); + class_key = ptr_bounds_clip (class_key, class_keysize); esprintf (name_key, "%s.%s", SSDATA (Vinvocation_name), attribute); sprintf (class_key, "%s.%s", EMACS_CLASS, class); diff --git a/src/fringe.c b/src/fringe.c index 087ef33434..a558117374 100644 --- a/src/fringe.c +++ b/src/fringe.c @@ -24,6 +24,7 @@ along with GNU Emacs. If not, see . */ #include "lisp.h" #include "frame.h" +#include "ptr-bounds.h" #include "window.h" #include "dispextern.h" #include "buffer.h" @@ -1591,7 +1592,9 @@ If BITMAP already exists, the existing definition is replaced. */) fb.dynamic = true; xfb = xmalloc (sizeof fb + fb.height * BYTES_PER_BITMAP_ROW); - fb.bits = b = (unsigned short *) (xfb + 1); + fb.bits = b = ((unsigned short *) + ptr_bounds_clip (xfb + 1, fb.height * BYTES_PER_BITMAP_ROW)); + xfb = ptr_bounds_clip (xfb, sizeof *xfb); memset (b, 0, fb.height); j = 0; diff --git a/src/gmalloc.c b/src/gmalloc.c index 97ab76561f..99cb967e53 100644 --- a/src/gmalloc.c +++ b/src/gmalloc.c @@ -203,7 +203,8 @@ extern size_t _bytes_free; /* Internal versions of `malloc', `realloc', and `free' used when these functions need to call each other. - They are the same but don't call the hooks. */ + They are the same but don't call the hooks + and don't bound the resulting pointers. */ extern void *_malloc_internal (size_t); extern void *_realloc_internal (void *, size_t); extern void _free_internal (void *); @@ -921,7 +922,8 @@ malloc (size_t size) among multiple threads. We just leave it for compatibility with glibc malloc (i.e., assignments to gmalloc_hook) for now. */ hook = gmalloc_hook; - return (hook != NULL ? *hook : _malloc_internal) (size); + void *result = (hook ? hook : _malloc_internal) (size); + return ptr_bounds_clip (result, size); } #if !(defined (_LIBC) || defined (HYBRID_MALLOC)) @@ -1434,7 +1436,8 @@ realloc (void *ptr, size_t size) return NULL; hook = grealloc_hook; - return (hook != NULL ? *hook : _realloc_internal) (ptr, size); + void *result = (hook ? hook : _realloc_internal) (ptr, size); + return ptr_bounds_clip (result, size); } /* Copyright (C) 1991, 1992, 1994 Free Software Foundation, Inc. @@ -1608,6 +1611,7 @@ aligned_alloc (size_t alignment, size_t size) { l->exact = result; result = l->aligned = (char *) result + adj; + result = ptr_bounds_clip (result, size); } UNLOCK_ALIGNED_BLOCKS (); if (l == NULL) diff --git a/src/lisp.h b/src/lisp.h index 38a098da82..32e2208558 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -916,9 +916,14 @@ INLINE Lisp_Object make_lisp_symbol (struct Lisp_Symbol *sym) { #ifdef __CHKP__ - char *symoffset = (char *) sym - (intptr_t) lispsym; + /* Although this should use '__builtin___bnd_narrow_ptr_bounds (sym, + sym, sizeof *sym)', that would run afoul of GCC bug 83251 + . */ + char *addr = __builtin___bnd_set_ptr_bounds (sym, sizeof *sym); + char *symoffset = addr - (intptr_t) lispsym; #else - /* If !__CHKP__ this is equivalent, and is a bit faster as of GCC 7. */ + /* If !__CHKP__, GCC 7 x86-64 generates faster code if lispsym is + cast to char * rather than to intptr_t. */ char *symoffset = (char *) ((char *) sym - (char *) lispsym); #endif Lisp_Object a = TAG_PTR (Lisp_Symbol, symoffset); diff --git a/src/ptr-bounds.h b/src/ptr-bounds.h index 54979824c0..76740da3d3 100644 --- a/src/ptr-bounds.h +++ b/src/ptr-bounds.h @@ -17,6 +17,18 @@ GNU General Public License for more details. You should have received a copy of the GNU General Public License along with GNU Emacs. If not, see . */ +/* Pointer bounds checking is a no-op unless running on hardware + supporting Intel MPX (Intel Skylake or better). Also, it requires + GCC 5 and Linux kernel 3.19, or later. Configure with + CFLAGS='-fcheck-pointer-bounds -mmpx', perhaps with + -fchkp-first-field-has-own-bounds thrown in. + + Although pointer bounds checking can help during debugging, it is + disabled by default because it hurts performance significantly. + The checking does not detect all pointer errors. For example, a + dumped Emacs might not detect a bounds violation of a pointer that + was created before Emacs was dumped. */ + #ifndef PTR_BOUNDS_H #define PTR_BOUNDS_H @@ -26,6 +38,19 @@ along with GNU Emacs. If not, see . */ return their first argument. These macros return either void *, or the same type as their first argument. */ +INLINE_HEADER_BEGIN + +/* Return a copy of P, with bounds narrowed to [P, P + N). */ +#ifdef __CHKP__ +INLINE void * +ptr_bounds_clip (void const *p, size_t n) +{ + return __builtin___bnd_narrow_ptr_bounds (p, p, n); +} +#else +# define ptr_bounds_clip(p, n) ((void) (size_t) {n}, p) +#endif + /* Return a copy of P, but with the bounds of Q. */ #ifdef __CHKP__ # define ptr_bounds_copy(p, q) __builtin___bnd_copy_ptr_bounds (p, q) @@ -49,4 +74,6 @@ along with GNU Emacs. If not, see . */ # define ptr_bounds_set(p, n) ((void) (size_t) {n}, p) #endif +INLINE_HEADER_END + #endif /* PTR_BOUNDS_H */ -- 2.14.1