svn commit: r355780 - in head/sys: arm/arm arm/include kern sys

Ian Lepore ian at FreeBSD.org
Sun Dec 15 21:16:36 UTC 2019


Author: ian
Date: Sun Dec 15 21:16:35 2019
New Revision: 355780
URL: https://svnweb.freebsd.org/changeset/base/355780

Log:
  Rewrite arm kernel stack unwind code to work when unwinding through modules.
  
  The arm kernel stack unwinder has apparently never been able to unwind when
  the path of execution leads through a kernel module. There was code that
  tried to handle modules by looking for the unwind data in them, but it did
  so by trying to find symbols which have never existed in arm kernel
  modules. That caused the unwind code to panic, and because part of panic
  handling calls into the unwind code, that just created a recursion loop.
  
  Locating the unwind data in a loaded module requires accessing the Elf
  section headers to find the SHT_ARM_EXIDX section. For preloaded modules
  those headers are present in a metadata blob. For dynamically loaded
  modules, the headers are present only while the loading is in progress; the
  memory is freed once the module is ready to use. For that reason, there is
  new code in kern/link_elf.c, wrapped in #ifdef __arm__, to extract the
  unwind info while the headers are loaded. The values are saved into new
  fields in the linker_file structure which are also conditional on __arm__.
  
  In arm/unwind.c there is new code to locally cache the per-module info
  needed to find the unwind tables. The local cache is crafted for lockless
  read access, because the unwind code often needs to run in context where
  sleeping is not allowed.  A large comment block describes the local cache
  list, so I won't repeat it all here.

Modified:
  head/sys/arm/arm/elf_machdep.c
  head/sys/arm/arm/unwind.c
  head/sys/arm/include/stack.h
  head/sys/kern/kern_linker.c
  head/sys/kern/link_elf.c
  head/sys/sys/linker.h

Modified: head/sys/arm/arm/elf_machdep.c
==============================================================================
--- head/sys/arm/arm/elf_machdep.c	Sun Dec 15 21:11:15 2019	(r355779)
+++ head/sys/arm/arm/elf_machdep.c	Sun Dec 15 21:16:35 2019	(r355780)
@@ -47,6 +47,7 @@ __FBSDID("$FreeBSD$");
 
 #include <machine/elf.h>
 #include <machine/md_var.h>
+#include <machine/stack.h>
 #ifdef VFP
 #include <machine/vfp.h>
 #endif
@@ -309,12 +310,21 @@ elf_cpu_load_file(linker_file_t lf)
 	cpu_l2cache_wb_range((vm_offset_t)lf->address, (vm_size_t)lf->size);
 	cpu_icache_sync_range((vm_offset_t)lf->address, (vm_size_t)lf->size);
 #endif
+
+	/*
+	 * Inform the stack(9) code of the new module, so it can acquire its
+	 * per-module unwind data.
+	 */
+	unwind_module_loaded(lf);
+
 	return (0);
 }
 
 int
-elf_cpu_unload_file(linker_file_t lf __unused)
+elf_cpu_unload_file(linker_file_t lf)
 {
 
+	/* Inform the stack(9) code that this module is gone. */
+	unwind_module_unloaded(lf);
 	return (0);
 }

Modified: head/sys/arm/arm/unwind.c
==============================================================================
--- head/sys/arm/arm/unwind.c	Sun Dec 15 21:11:15 2019	(r355779)
+++ head/sys/arm/arm/unwind.c	Sun Dec 15 21:16:35 2019	(r355780)
@@ -32,8 +32,11 @@
 __FBSDID("$FreeBSD$");
 
 #include <sys/param.h>
-#include <sys/systm.h>
+#include <sys/kernel.h>
 #include <sys/linker.h>
+#include <sys/malloc.h>
+#include <sys/queue.h>
+#include <sys/systm.h>
 
 #include <machine/machdep.h>
 #include <machine/stack.h>
@@ -94,79 +97,236 @@ struct unwind_idx {
 	uint32_t insn;
 };
 
-/* Expand a 31-bit signed value to a 32-bit signed value */
-static __inline int32_t
-expand_prel31(uint32_t prel31)
+/*
+ * Local cache of unwind info for loaded modules.
+ *
+ * To unwind the stack through the code in a loaded module, we need to access
+ * the module's exidx unwind data.  To locate that data, one must search the
+ * elf section headers for the SHT_ARM_EXIDX section.  Those headers are
+ * available at the time the module is being loaded, but are discarded by time
+ * the load process has completed.  Code in kern/link_elf.c locates the data we
+ * need and stores it into the linker_file structure before calling the arm
+ * machdep routine for handling loaded modules (in arm/elf_machdep.c).  That
+ * function calls into this code to pass along the unwind info, which we save
+ * into one of these module_info structures.
+ *
+ * Because we have to help stack(9) gather stack info at any time, including in
+ * contexts where sleeping is not allowed, we cannot use linker_file_foreach()
+ * to walk the kernel's list of linker_file structs, because doing so requires
+ * acquiring an exclusive sx_lock.  So instead, we keep a local list of these
+ * structures, one for each loaded module (and one for the kernel itself that we
+ * synthesize at init time).  New entries are added to the end of this list as
+ * needed, but entries are never deleted from the list.  Instead, they are
+ * cleared out in-place to mark them as unused.  That means the code doing stack
+ * unwinding can always safely walk the list without locking, because the
+ * structure of the list never changes in a way that would cause the walker to
+ * follow a bad link.
+ *
+ * A cleared-out entry on the list has module start=UINTPTR_MAX and end=0, so
+ * start <= addr < end cannot be true for any value of addr being searched for.
+ * We also don't have to worry about races where we look up the unwind info just
+ * before a module is unloaded and try to access it concurrently with or just
+ * after the unloading happens in another thread, because that means the path of
+ * execution leads through a now-unloaded module, and that's already well into
+ * undefined-behavior territory.
+ *
+ * List entries marked as unused get reused when new modules are loaded.  We
+ * don't worry about holding a few unused bytes of memory in the list after
+ * unloading a module.
+ */
+struct module_info {
+	uintptr_t	module_start;   /* Start of loaded module */
+	uintptr_t	module_end;     /* End of loaded module */
+	uintptr_t	exidx_start;    /* Start of unwind data */
+	uintptr_t	exidx_end;      /* End of unwind data */
+	STAILQ_ENTRY(module_info)
+			link;           /* Link to next entry */
+};
+static STAILQ_HEAD(, module_info) module_list;
+
+/*
+ * Hide ugly casting in somewhat-less-ugly macros.
+ *  CADDR - cast a pointer or number to caddr_t.
+ *  UADDR - cast a pointer or number to uintptr_t.
+ */
+#define	CADDR(addr)	((caddr_t)(void*)(uintptr_t)(addr))
+#define	UADDR(addr)	((uintptr_t)(addr))
+
+/*
+ * Clear the info in an existing module_info entry on the list.  The
+ * module_start/end addresses are set to values that cannot match any real
+ * memory address.  The entry remains on the list, but will be ignored until it
+ * is populated with new data.
+ */
+static void
+clear_module_info(struct module_info *info)
 {
+	info->module_start = UINTPTR_MAX;
+	info->module_end   = 0;
+}
 
-	return ((int32_t)(prel31 & 0x7fffffffu) << 1) / 2;
+/*
+ * Populate an existing module_info entry (which is already on the list) with
+ * the info for a new module.
+ */
+static void
+populate_module_info(struct module_info *info, linker_file_t lf)
+{
+
+	/*
+	 * Careful!  The module_start and module_end fields must not be set
+	 * until all other data in the structure is valid.
+	 */
+	info->exidx_start  = UADDR(lf->exidx_addr);
+	info->exidx_end    = UADDR(lf->exidx_addr) + lf->exidx_size;
+	info->module_start = UADDR(lf->address);
+	info->module_end   = UADDR(lf->address) + lf->size;
 }
 
-struct search_context {
-	uint32_t addr;
-	caddr_t exidx_start;
-	caddr_t exidx_end;
-};
+/*
+ * Create a new empty module_info entry and add it to the tail of the list.
+ */
+static struct module_info *
+create_module_info(void)
+{
+	struct module_info *info;
 
-static int
-module_search(linker_file_t lf, void *context)
+	info = malloc(sizeof(*info), M_CACHE, M_WAITOK | M_ZERO);
+	clear_module_info(info);
+	STAILQ_INSERT_TAIL(&module_list, info, link);
+	return (info);
+}
+
+/*
+ * Search for a module_info entry on the list whose address range contains the
+ * given address.  If the search address is zero (no module will be loaded at
+ * zero), then we're looking for an empty item to reuse, which is indicated by
+ * module_start being set to UINTPTR_MAX in the entry.
+ */
+static struct module_info *
+find_module_info(uintptr_t addr)
 {
-	struct search_context *sc = context;
-	linker_symval_t symval;
-	c_linker_sym_t sym;
+	struct module_info *info;
 
-	if (lf->address <= (caddr_t)sc->addr &&
-	    (lf->address + lf->size) >= (caddr_t)sc->addr) {
-		if ((LINKER_LOOKUP_SYMBOL(lf, "__exidx_start", &sym) == 0 ||
-		    LINKER_LOOKUP_SYMBOL(lf, "exidx_start", &sym) == 0) &&
-		    LINKER_SYMBOL_VALUES(lf, sym, &symval) == 0)
-			sc->exidx_start = symval.value;
+	STAILQ_FOREACH(info, &module_list, link) {
+		if ((addr >= info->module_start && addr < info->module_end) ||
+		    (addr == 0 && info->module_start == UINTPTR_MAX))
+			return (info);
+	}
+	return (NULL);
+}
 
-		if ((LINKER_LOOKUP_SYMBOL(lf, "__exidx_end", &sym) == 0 ||
-		    LINKER_LOOKUP_SYMBOL(lf, "exidx_end", &sym) == 0) &&
-		    LINKER_SYMBOL_VALUES(lf, sym, &symval) == 0)
-			sc->exidx_end = symval.value;
+/*
+ * Handle the loading of a new module by populating a module_info for it.  This
+ * is called for both preloaded and dynamically loaded modules.
+ */
+void
+unwind_module_loaded(struct linker_file *lf)
+{
+	struct module_info *info;
 
-		if (sc->exidx_start != NULL && sc->exidx_end != NULL)
-			return (1);
-		panic("Invalid module %s, no unwind tables\n", lf->filename);
+	/*
+	 * A module that contains only data may have no unwind info; don't
+	 * create any module info for it.
+	 */
+	if (lf->exidx_size == 0)
+		return;
+
+	/*
+	 * Find an unused entry in the existing list to reuse.  If we don't find
+	 * one, create a new one and link it into the list.  This is the only
+	 * place the module_list is modified.  Adding a new entry to the list
+	 * will not perturb any other threads currently walking the list.  This
+	 * function is invoked while kern_linker is still holding its lock
+	 * to prevent its module list from being modified, so we don't have to
+	 * worry about racing other threads doing an insert concurrently.
+	 */
+	if ((info = find_module_info(0)) == NULL) {
+		info = create_module_info();
 	}
+	populate_module_info(info, lf);
+}
+
+/* Handle the unloading of a module. */
+void
+unwind_module_unloaded(struct linker_file *lf)
+{
+	struct module_info *info;
+
+	/*
+	 * A module that contains only data may have no unwind info and there
+	 * won't be a list entry for it.
+	 */
+	if (lf->exidx_size == 0)
+		return;
+
+	/*
+	 * When a module is unloaded, we clear the info out of its entry in the
+	 * module list, making that entry available for later reuse.
+	 */
+	if ((info = find_module_info(UADDR(lf->address))) == NULL) {
+		printf("arm unwind: module '%s' not on list at unload time\n",
+		    lf->filename);
+		return;
+	}
+	clear_module_info(info);
+}
+
+/*
+ * Initialization must run fairly early, as soon as malloc(9) is available, and
+ * definitely before witness, which uses stack(9).  We synthesize a module_info
+ * entry for the kernel, because unwind_module_loaded() doesn't get called for
+ * it.  Also, it is unlike other modules in that the elf metadata for locating
+ * the unwind tables might be stripped, so instead we have to use the
+ * _exidx_start/end symbols created by ldscript.arm.
+ */
+static int
+module_info_init(void *arg __unused)
+{
+	struct linker_file thekernel;
+
+	STAILQ_INIT(&module_list);
+
+	thekernel.filename   = "kernel";
+	thekernel.address    = CADDR(&_start);
+	thekernel.size       = UADDR(&_end) - UADDR(&_start);
+	thekernel.exidx_addr = CADDR(&_exidx_start);
+	thekernel.exidx_size = UADDR(&_exidx_end) - UADDR(&_exidx_start);
+	populate_module_info(create_module_info(), &thekernel);
+
 	return (0);
 }
+SYSINIT(unwind_init, SI_SUB_KMEM, SI_ORDER_ANY, module_info_init, NULL);
 
+/* Expand a 31-bit signed value to a 32-bit signed value */
+static __inline int32_t
+expand_prel31(uint32_t prel31)
+{
+
+	return ((int32_t)(prel31 & 0x7fffffffu) << 1) / 2;
+}
+
 /*
  * Perform a binary search of the index table to find the function
  * with the largest address that doesn't exceed addr.
  */
 static struct unwind_idx *
-find_index(uint32_t addr, int search_modules)
+find_index(uint32_t addr)
 {
-	struct search_context sc;
-	caddr_t idx_start, idx_end;
+	struct module_info *info;
 	unsigned int min, mid, max;
 	struct unwind_idx *start;
 	struct unwind_idx *item;
 	int32_t prel31_addr;
 	uint32_t func_addr;
 
-	start = (struct unwind_idx *)&_exidx_start;
-	idx_start = (caddr_t)&_exidx_start;
-	idx_end = (caddr_t)&_exidx_end;
+	info = find_module_info(addr);
+	if (info == NULL)
+		return NULL;
 
-	/* This may acquire a lock */
-	if (search_modules) {
-		bzero(&sc, sizeof(sc));
-		sc.addr = addr;
-		if (linker_file_foreach(module_search, &sc) != 0 &&
-		   sc.exidx_start != NULL && sc.exidx_end != NULL) {
-			start = (struct unwind_idx *)sc.exidx_start;
-			idx_start = sc.exidx_start;
-			idx_end = sc.exidx_end;
-		}
-	}
-
 	min = 0;
-	max = (idx_end - idx_start) / sizeof(struct unwind_idx);
+	max = (info->exidx_end - info->exidx_start) / sizeof(struct unwind_idx);
+	start = (struct unwind_idx *)CADDR(info->exidx_start);
 
 	while (min != max) {
 		mid = min + (max - min + 1) / 2;
@@ -377,11 +537,17 @@ unwind_tab(struct unwind_state *state)
 	return 0;
 }
 
+/*
+ * Unwind a single stack frame.
+ * Return 0 on success or 1 if the stack cannot be unwound any further.
+ *
+ * XXX The can_lock argument is no longer germane; a sweep of callers should be
+ * made to remove it after this new code has proven itself for a while.
+ */
 int
-unwind_stack_one(struct unwind_state *state, int can_lock)
+unwind_stack_one(struct unwind_state *state, int can_lock __unused)
 {
 	struct unwind_idx *index;
-	int finished;
 
 	/* Reset the mask of updated registers */
 	state->update_mask = 0;
@@ -390,26 +556,20 @@ unwind_stack_one(struct unwind_state *state, int can_l
 	state->start_pc = state->registers[PC];
 
 	/* Find the item to run */
-	index = find_index(state->start_pc, can_lock);
+	index = find_index(state->start_pc);
+	if (index == NULL || index->insn == EXIDX_CANTUNWIND)
+		return 1;
 
-	finished = 0;
-	if (index->insn != EXIDX_CANTUNWIND) {
-		if (index->insn & (1U << 31)) {
-			/* The data is within the instruction */
-			state->insn = &index->insn;
-		} else {
-			/* A prel31 offset to the unwind table */
-			state->insn = (uint32_t *)
-			    ((uintptr_t)&index->insn +
-			     expand_prel31(index->insn));
-		}
-		/* Run the unwind function */
-		finished = unwind_tab(state);
+	if (index->insn & (1U << 31)) {
+		/* The data is within the instruction */
+		state->insn = &index->insn;
+	} else {
+		/* A prel31 offset to the unwind table */
+		state->insn = (uint32_t *)
+		    ((uintptr_t)&index->insn +
+		     expand_prel31(index->insn));
 	}
 
-	/* This is the top of the stack, finish */
-	if (index->insn == EXIDX_CANTUNWIND)
-		finished = 1;
-
-	return (finished);
+	/* Run the unwind function, return its finished/not-finished status. */
+	return (unwind_tab(state));
 }

Modified: head/sys/arm/include/stack.h
==============================================================================
--- head/sys/arm/include/stack.h	Sun Dec 15 21:11:15 2019	(r355779)
+++ head/sys/arm/include/stack.h	Sun Dec 15 21:16:35 2019	(r355780)
@@ -55,6 +55,14 @@ struct unwind_state {
 #define	LR	14
 #define	PC	15
 
+#ifdef _KERNEL
+
 int unwind_stack_one(struct unwind_state *, int);
+
+struct linker_file;
+void unwind_module_loaded(struct linker_file *);
+void unwind_module_unloaded(struct linker_file *);
+
+#endif
 
 #endif /* !_MACHINE_STACK_H_ */

Modified: head/sys/kern/kern_linker.c
==============================================================================
--- head/sys/kern/kern_linker.c	Sun Dec 15 21:11:15 2019	(r355779)
+++ head/sys/kern/kern_linker.c	Sun Dec 15 21:16:35 2019	(r355780)
@@ -624,6 +624,10 @@ linker_make_file(const char *pathname, linker_class_t 
 	lf->ndeps = 0;
 	lf->deps = NULL;
 	lf->loadcnt = ++loadcnt;
+#ifdef __arm__
+	lf->exidx_addr = 0;
+	lf->exidx_size = 0;
+#endif
 	STAILQ_INIT(&lf->common);
 	TAILQ_INIT(&lf->modules);
 	TAILQ_INSERT_TAIL(&linker_files, lf, link);

Modified: head/sys/kern/link_elf.c
==============================================================================
--- head/sys/kern/link_elf.c	Sun Dec 15 21:11:15 2019	(r355779)
+++ head/sys/kern/link_elf.c	Sun Dec 15 21:16:35 2019	(r355780)
@@ -773,6 +773,50 @@ preload_protect(elf_file_t ef, vm_prot_t prot)
 #endif
 }
 
+#ifdef __arm__
+/*
+ * Locate the ARM exception/unwind table info for DDB and stack(9) use by
+ * searching for the section header that describes it.  There may be no unwind
+ * info, for example in a module containing only data.
+ */
+static void
+link_elf_locate_exidx(linker_file_t lf, Elf_Shdr *shdr, int nhdr)
+{
+	int i;
+
+	for (i = 0; i < nhdr; i++) {
+		if (shdr[i].sh_type == SHT_ARM_EXIDX) {
+			lf->exidx_addr = shdr[i].sh_addr + lf->address;
+			lf->exidx_size = shdr[i].sh_size;
+			break;
+		}
+	}
+}
+
+/*
+ * Locate the section headers metadata in a preloaded module, then use it to
+ * locate the exception/unwind table in the module.  The size of the metadata
+ * block is stored in a uint32 word immediately before the data itself, and a
+ * comment in preload_search_info() says it is safe to rely on that.
+ */
+static void
+link_elf_locate_exidx_preload(struct linker_file *lf, caddr_t modptr)
+{
+	uint32_t *modinfo;
+	Elf_Shdr *shdr;
+	uint32_t  nhdr;
+
+	modinfo = (uint32_t *)preload_search_info(modptr,
+	    MODINFO_METADATA | MODINFOMD_SHDR);
+	if (modinfo != NULL) {
+		shdr = (Elf_Shdr *)modinfo;
+		nhdr = modinfo[-1] / sizeof(Elf_Shdr);
+		link_elf_locate_exidx(lf, shdr, nhdr);
+	}
+}
+
+#endif /* __arm__ */
+
 static int
 link_elf_link_preload(linker_class_t cls, const char *filename,
     linker_file_t *result)
@@ -828,6 +872,10 @@ link_elf_link_preload(linker_class_t cls, const char *
 		lf->ctors_size = *ctors_sizep;
 	}
 
+#ifdef __arm__
+	link_elf_locate_exidx_preload(lf, modptr);
+#endif
+
 	error = parse_dynamic(ef);
 	if (error == 0)
 		error = parse_dpcpu(ef);
@@ -1225,6 +1273,11 @@ link_elf_load_file(linker_class_t cls, const char* fil
 	ef->ddbstrtab = ef->strbase;
 
 nosyms:
+
+#ifdef __arm__
+	link_elf_locate_exidx(lf, shdr, hdr->e_shnum);
+#endif
+
 	error = link_elf_link_common_finish(lf);
 	if (error != 0)
 		goto out;

Modified: head/sys/sys/linker.h
==============================================================================
--- head/sys/sys/linker.h	Sun Dec 15 21:11:15 2019	(r355779)
+++ head/sys/sys/linker.h	Sun Dec 15 21:16:35 2019	(r355780)
@@ -97,6 +97,11 @@ struct linker_file {
      */
     int			nenabled;	/* number of enabled probes. */
     int			fbt_nentries;	/* number of fbt entries created. */
+
+#ifdef __arm__
+    caddr_t		exidx_addr;	/* Unwind data index table start */
+    size_t		exidx_size;	/* Unwind data index table size */
+#endif
 };
 
 /*


More information about the svn-src-all mailing list