From bd1e1f8488196e67decb7c848aeb5623e9887cfb Mon Sep 17 00:00:00 2001 From: Daniel Dorau Date: Wed, 8 Jan 2020 10:01:39 +0100 Subject: [PATCH] =?UTF-8?q?Lauff=C3=A4higkeit=20mit=20valgrind=20herstelle?= =?UTF-8?q?n?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Startet valgrind unter musl, gibt es einen segmentation fault im durch valgrind ersetzten free(). Der Absturz resultiert aus dem Zugriff auf die noch nicht relokierte Variable init_done in valgrinds coregrind/m_replacemalloc/vg_replace_malloc.c:184 auf die per Makro aus dem ersetzten free() zugegriffen wird. Der Aufruf des free erfolgte hierbei aus der Funktion map_library() in musl, ldso/dynlink.c:762. Ursache ist das Zusammenspiel aus dem loader von musl und dem Funktionsersetzungsmechanismus von valgrind. Die memorychecker Funktionen von valgrind (vgpreload_memcheck-mips32-linux.so) werden durch den loader der musl als shared library geladen. Die Funktionsersetzung von valgrind basiert jedoch nicht auf den Standardmechanismen des loaders, sondern auf einer unabhängigen Implementation, die Symbolnamen gegen die zu ersetzenden Symbole matcht, wobei im neuen Symbolnamen die library als auch Funktion als Pattern enthalten sind. (vgl. http://valgrind.org/docs/manual/manual-core-adv.html#manual-core-adv.wrapping) Als Hook um die Symbolersetzung durchzuführen fängt valgrind den mmap syscall ab um vom Laden einer shared library informiert zu werden. Im konkreten Fall der musl bedeutet das, dass die Ersetzung der malloc/free Funktionen von valgrind vorgenommen werden bevor musl mit dem Laden der shared library fertig ist und alle Relokationen durchgeführt hat. Dies resultiert dann darin, dass aus dynlink.c bereits das neue free() aufgerufen wird, noch bevor die Relokation der Symbole innerhalb von vg_replace_malloc.c abgeschlossen ist, was dann zum Absturz führt. Zudem führt die Ersetzung zu einer Inkonsistenz von durch musl allokiertem Speicher in dynlink.c, der ggf. später durch den Allokator von valgrind versucht wird freizugeben. Diese Inkonsistenz wird sich evtl. nicht vollständig vermeiden lassen. Lösungsansatz dieses Patches ist es, dass aus musls dylink.c immer die malloc/free Implementation von musl verwendet wird und somit eine Ersetzung von malloc/free durch valgrind für die Aufrufe aus dynlink.c folgenlos bleibt. Hierfür werden die malloc/free Funktionen von musl in musl_* Aufrufe gekapselt. --- include/stdlib.h | 4 ++++ ldso/dynlink.c | 50 ++++++++++++++++++++++----------------------- src/malloc/malloc.c | 33 ++++++++++++++++++++++-------- 3 files changed, 53 insertions(+), 34 deletions(-) Index: b/include/stdlib.h =================================================================== --- a/include/stdlib.h +++ b/include/stdlib.h @@ -36,9 +36,13 @@ int rand (void); void srand (unsigned); void *malloc (size_t); +void *musl_malloc (size_t); void *calloc (size_t, size_t); +void *musl_calloc (size_t, size_t); void *realloc (void *, size_t); +void *musl_realloc (void *, size_t); void free (void *); +void musl_free (void *); void *aligned_alloc(size_t, size_t); _Noreturn void abort (void); Index: b/ldso/dynlink.c =================================================================== --- a/ldso/dynlink.c +++ b/ldso/dynlink.c @@ -463,7 +463,7 @@ static void do_relocs(struct dso *dso, s case REL_TLSDESC: if (stride<3) addend = reloc_addr[1]; if (def.dso->tls_id > static_tls_cnt) { - struct td_index *new = malloc(sizeof *new); + struct td_index *new = musl_malloc(sizeof *new); if (!new) { error( "Error relocating %s: cannot allocate TLSDESC for %s", @@ -516,7 +516,7 @@ static void redo_lazy_relocs() p->lazy_next = lazy_head; lazy_head = p; } else { - free(p->lazy); + musl_free(p->lazy); p->lazy = 0; p->lazy_next = 0; } @@ -589,7 +589,7 @@ static void unmap_library(struct dso *ds munmap((void *)dso->loadmap->segs[i].addr, dso->loadmap->segs[i].p_memsz); } - free(dso->loadmap); + musl_free(dso->loadmap); } else if (dso->map && dso->map_len) { munmap(dso->map, dso->map_len); } @@ -619,7 +619,7 @@ static void *map_library(int fd, struct goto noexec; phsize = eh->e_phentsize * eh->e_phnum; if (phsize > sizeof buf - sizeof *eh) { - allocated_buf = malloc(phsize); + allocated_buf = musl_malloc(phsize); if (!allocated_buf) return 0; l = pread(fd, allocated_buf, phsize, eh->e_phoff); if (l < 0) goto error; @@ -666,7 +666,7 @@ static void *map_library(int fd, struct } if (!dyn) goto noexec; if (DL_FDPIC && !(eh->e_flags & FDPIC_CONSTDISP_FLAG)) { - dso->loadmap = calloc(1, sizeof *dso->loadmap + dso->loadmap = musl_calloc(1, sizeof *dso->loadmap + nsegs * sizeof *dso->loadmap->segs); if (!dso->loadmap) goto error; dso->loadmap->nsegs = nsegs; @@ -771,13 +771,13 @@ done_mapping: dso->base = base; dso->dynv = laddr(dso, dyn); if (dso->tls.size) dso->tls.image = laddr(dso, tls_image); - free(allocated_buf); + musl_free(allocated_buf); return map; noexec: errno = ENOEXEC; error: if (map!=MAP_FAILED) unmap_library(dso); - free(allocated_buf); + musl_free(allocated_buf); return 0; } @@ -866,7 +866,7 @@ static int fixup_rpath(struct dso *p, ch /* Disallow non-absolute origins for suid/sgid/AT_SECURE. */ if (libc.secure && *origin != '/') return 0; - p->rpath = malloc(strlen(p->rpath_orig) + n*l + 1); + p->rpath = musl_malloc(strlen(p->rpath_orig) + n*l + 1); if (!p->rpath) return -1; d = p->rpath; @@ -945,7 +945,7 @@ static void makefuncdescs(struct dso *p) p->funcdescs = dl_mmap(size); self_done = 1; } else { - p->funcdescs = malloc(size); + p->funcdescs = musl_malloc(size); } if (!p->funcdescs) { if (!runtime) a_crash(); @@ -1058,7 +1058,7 @@ static struct dso *load_library(const ch FILE *f = fopen(etc_ldso_path, "rbe"); if (f) { if (getdelim(&sys_path, (size_t[1]){0}, 0, f) <= 0) { - free(sys_path); + musl_free(sys_path); sys_path = ""; } fclose(f); @@ -1119,7 +1119,7 @@ static struct dso *load_library(const ch if (n_th > SSIZE_MAX / per_th) alloc_size = SIZE_MAX; else alloc_size += n_th * per_th; } - p = calloc(1, alloc_size); + p = musl_calloc(1, alloc_size); if (!p) { unmap_library(&temp_dso); return 0; @@ -1179,7 +1179,7 @@ static void load_direct_deps(struct dso /* Use builtin buffer for apps with no external deps, to * preserve property of no runtime failure paths. */ p->deps = (p==head && cnt<2) ? builtin_deps : - calloc(cnt+1, sizeof *p->deps); + musl_calloc(cnt+1, sizeof *p->deps); if (!p->deps) { error("Error loading dependencies for %s", p->name); if (runtime) longjmp(*rtld_fail, 1); @@ -1236,8 +1236,8 @@ static void extend_bfs_deps(struct dso * for (j=cnt=0; jndeps_direct; j++) if (!dep->deps[j]->mark) cnt++; tmp = no_realloc ? - malloc(sizeof(*tmp) * (ndeps_all+cnt+1)) : - realloc(p->deps, sizeof(*tmp) * (ndeps_all+cnt+1)); + musl_malloc(sizeof(*tmp) * (ndeps_all+cnt+1)) : + musl_realloc(p->deps, sizeof(*tmp) * (ndeps_all+cnt+1)); if (!tmp) { error("Error recording dependencies for %s", p->name); if (runtime) longjmp(*rtld_fail, 1); @@ -1422,7 +1422,7 @@ static struct dso **queue_ctors(struct d if (dso==head && cnt <= countof(builtin_ctor_queue)) queue = builtin_ctor_queue; else - queue = calloc(cnt, sizeof *queue); + queue = musl_calloc(cnt, sizeof *queue); if (!queue) { error("Error allocating constructor queue: %m\n"); @@ -1505,7 +1505,7 @@ void __libc_start_init(void) { do_init_fini(main_ctor_queue); if (!__malloc_replaced && main_ctor_queue != builtin_ctor_queue) - free(main_ctor_queue); + musl_free(main_ctor_queue); main_ctor_queue = 0; } @@ -1891,7 +1891,7 @@ void __dls3(size_t *sp) update_tls_size(); void *initial_tls = builtin_tls; if (libc.tls_size > sizeof builtin_tls || tls_align > MIN_TLS_ALIGN) { - initial_tls = calloc(libc.tls_size, 1); + initial_tls = musl_calloc(libc.tls_size, 1); if (!initial_tls) { dprintf(2, "%s: Error getting %zu bytes thread-local storage: %m\n", argv[0], libc.tls_size); @@ -1964,7 +1964,7 @@ static void prepare_lazy(struct dso *p) size_t i=0; search_vec(p->dynv, &i, DT_MIPS_SYMTABNO); n += i-j; } - p->lazy = calloc(n, 3*sizeof(size_t)); + p->lazy = musl_calloc(n, 3*sizeof(size_t)); if (!p->lazy) { error("Error preparing lazy relocation for %s: %m", p->name); longjmp(*rtld_fail, 1); @@ -2011,17 +2011,17 @@ void *dlopen(const char *file, int mode) next = p->next; while (p->td_index) { void *tmp = p->td_index->next; - free(p->td_index); + musl_free(p->td_index); p->td_index = tmp; } - free(p->funcdescs); + musl_free(p->funcdescs); if (p->rpath != p->rpath_orig) - free(p->rpath); - free(p->deps); + musl_free(p->rpath); + musl_free(p->deps); unmap_library(p); - free(p); + musl_free(p); } - free(ctor_queue); + musl_free(ctor_queue); ctor_queue = 0; if (!orig_tls_tail) libc.tls_head = 0; tls_tail = orig_tls_tail; @@ -2089,7 +2089,7 @@ end: pthread_rwlock_unlock(&lock); if (ctor_queue) { do_init_fini(ctor_queue); - free(ctor_queue); + musl_free(ctor_queue); } pthread_setcancelstate(cs, 0); return p; Index: b/src/malloc/malloc.c =================================================================== --- a/src/malloc/malloc.c +++ b/src/malloc/malloc.c @@ -281,7 +281,7 @@ static void trim(struct chunk *self, siz __bin_chunk(split); } -void *malloc(size_t n) +void *musl_malloc(size_t n) { struct chunk *c; int i, j; @@ -330,6 +330,10 @@ void *malloc(size_t n) return CHUNK_TO_MEM(c); } +void *malloc(size_t n) { + return musl_malloc(n); +} + static size_t mal0_clear(char *p, size_t pagesz, size_t n) { #ifdef __GNUC__ @@ -348,14 +352,14 @@ static size_t mal0_clear(char *p, size_t } } -void *calloc(size_t m, size_t n) +void *musl_calloc(size_t m, size_t n) { if (n && m > (size_t)-1/n) { errno = ENOMEM; return 0; } n *= m; - void *p = malloc(n); + void *p = musl_malloc(n); if (!p) return p; if (!__malloc_replaced) { if (IS_MMAPPED(MEM_TO_CHUNK(p))) @@ -366,13 +370,17 @@ void *calloc(size_t m, size_t n) return memset(p, 0, n); } -void *realloc(void *p, size_t n) +void *calloc(size_t m, size_t n) { + return musl_calloc(m, n); +} + +void *musl_realloc(void *p, size_t n) { struct chunk *self, *next; size_t n0, n1; void *new; - if (!p) return malloc(n); + if (!p) return musl_malloc(n); if (adjust_size(&n) < 0) return 0; @@ -386,7 +394,7 @@ void *realloc(void *p, size_t n) size_t newlen = n + extra; /* Crash on realloc of freed chunk */ if (extra & 1) a_crash(); - if (newlen < PAGE_SIZE && (new = malloc(n-OVERHEAD))) { + if (newlen < PAGE_SIZE && (new = musl_malloc(n-OVERHEAD))) { n0 = n; goto copy_free_ret; } @@ -429,13 +437,16 @@ void *realloc(void *p, size_t n) copy_realloc: /* As a last resort, allocate a new chunk and copy to it. */ - new = malloc(n-OVERHEAD); + new = musl_malloc(n-OVERHEAD); if (!new) return 0; copy_free_ret: memcpy(new, p, n0-OVERHEAD); - free(CHUNK_TO_MEM(self)); + musl_free(CHUNK_TO_MEM(self)); return new; } +void *realloc(void *p, size_t n) { + return musl_realloc(p, n); +} void __bin_chunk(struct chunk *self) { @@ -516,7 +527,7 @@ static void unmap_chunk(struct chunk *se __munmap(base, len); } -void free(void *p) +void musl_free(void *p) { if (!p) return; @@ -528,6 +539,10 @@ void free(void *p) __bin_chunk(self); } +void free(void *p) { + musl_free(p); +} + void __malloc_donate(char *start, char *end) { size_t align_start_up = (SIZE_ALIGN-1) & (-(uintptr_t)start - OVERHEAD);