From bd1e1f8488196e67decb7c848aeb5623e9887cfb Mon Sep 17 00:00:00 2001
From: Daniel Dorau <d.dorau@avm.de>
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; j<dep->ndeps_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);