Module Name: src
Committed By: riastradh
Date: Tue Sep 7 10:59:46 UTC 2021
Modified Files:
src/sys/kern: kern_ksyms.c
Log Message:
ksyms(4): Simply block unload until last /dev/ksyms close.
Otherwise, readers may get a garbled snapshot of ksyms (or a crash on
an assertion failure because of the garbled snapshot) if modules are
unloaded while they read.
https://mail-index.netbsd.org/source-changes-d/2021/08/17/msg013425.html
To generate a diff of this commit:
cvs rdiff -u -r1.98 -r1.99 src/sys/kern/kern_ksyms.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/kern/kern_ksyms.c
diff -u src/sys/kern/kern_ksyms.c:1.98 src/sys/kern/kern_ksyms.c:1.99
--- src/sys/kern/kern_ksyms.c:1.98 Sun Jul 18 06:57:28 2021
+++ src/sys/kern/kern_ksyms.c Tue Sep 7 10:59:46 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_ksyms.c,v 1.98 2021/07/18 06:57:28 mlelstv Exp $ */
+/* $NetBSD: kern_ksyms.c,v 1.99 2021/09/07 10:59:46 riastradh Exp $ */
/*-
* Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -73,7 +73,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_ksyms.c,v 1.98 2021/07/18 06:57:28 mlelstv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_ksyms.c,v 1.99 2021/09/07 10:59:46 riastradh Exp $");
#if defined(_KERNEL) && defined(_KERNEL_OPT)
#include "opt_copy_symtab.h"
@@ -117,6 +117,7 @@ static struct ksyms_symtab *ksyms_last_s
static bool ksyms_initted;
static bool ksyms_loaded;
static kmutex_t ksyms_lock __cacheline_aligned;
+static kcondvar_t ksyms_cv;
static struct ksyms_symtab kernel_symtab;
static void ksyms_hdr_init(const void *);
@@ -245,6 +246,7 @@ ksyms_init(void)
if (!ksyms_initted) {
mutex_init(&ksyms_lock, MUTEX_DEFAULT, IPL_NONE);
+ cv_init(&ksyms_cv, "ksyms");
ksyms_initted = true;
}
}
@@ -328,7 +330,6 @@ addsymtab(const char *name, void *symsta
tab->sd_minsym = UINTPTR_MAX;
tab->sd_maxsym = 0;
tab->sd_usroffset = 0;
- tab->sd_gone = false;
tab->sd_ctfstart = ctfstart;
tab->sd_ctfsize = ctfsize;
tab->sd_nmap = nmap;
@@ -446,9 +447,9 @@ addsymtab(const char *name, void *symsta
KASSERT(cold || mutex_owned(&ksyms_lock));
/*
- * Ensure ddb never witnesses an inconsistent state of the
- * queue, unless memory is so corrupt that we crash in
- * TAILQ_INSERT_TAIL.
+ * Publish the symtab. Do this at splhigh to ensure ddb never
+ * witnesses an inconsistent state of the queue, unless memory
+ * is so corrupt that we crash in TAILQ_INSERT_TAIL.
*/
s = splhigh();
TAILQ_INSERT_TAIL(&ksyms_symtabs, tab, sd_queue);
@@ -601,8 +602,6 @@ ksyms_getval_unlocked(const char *mod, c
#endif
TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) {
- if (__predict_false(st->sd_gone))
- continue;
if (mod != NULL && strcmp(st->sd_name, mod))
continue;
if ((es = findsym(sym, st, type)) != NULL) {
@@ -636,8 +635,6 @@ ksyms_get_mod(const char *mod)
mutex_enter(&ksyms_lock);
TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) {
- if (__predict_false(st->sd_gone))
- continue;
if (mod != NULL && strcmp(st->sd_name, mod))
continue;
break;
@@ -671,8 +668,6 @@ ksyms_mod_foreach(const char *mod, ksyms
/* find the module */
TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) {
- if (__predict_false(st->sd_gone))
- continue;
if (mod != NULL && strcmp(st->sd_name, mod))
continue;
@@ -716,8 +711,6 @@ ksyms_getname(const char **mod, const ch
return ENOENT;
TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) {
- if (st->sd_gone)
- continue;
if (v < st->sd_minsym || v > st->sd_maxsym)
continue;
sz = st->sd_symsize/sizeof(Elf_Sym);
@@ -780,37 +773,44 @@ void
ksyms_modunload(const char *name)
{
struct ksyms_symtab *st;
- bool do_free = false;
int s;
mutex_enter(&ksyms_lock);
TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) {
- if (st->sd_gone)
- continue;
if (strcmp(name, st->sd_name) != 0)
continue;
- st->sd_gone = true;
- ksyms_sizes_calc();
- if (ksyms_opencnt == 0) {
- /*
- * Ensure ddb never witnesses an inconsistent
- * state of the queue, unless memory is so
- * corrupt that we crash in TAILQ_REMOVE.
- */
- s = splhigh();
- TAILQ_REMOVE(&ksyms_symtabs, st, sd_queue);
- splx(s);
- do_free = true;
- }
break;
}
- mutex_exit(&ksyms_lock);
KASSERT(st != NULL);
- if (do_free) {
- kmem_free(st->sd_nmap, st->sd_nmapsize * sizeof(uint32_t));
- kmem_free(st, sizeof(*st));
- }
+ /*
+ * Wait for last /dev/ksyms close -- readers may be in the
+ * middle of viewing a snapshot including this module. (We
+ * could skip this if it's past ksyms_last_snapshot, but it's
+ * not clear that's worth the effort.)
+ */
+ while (ksyms_opencnt)
+ cv_wait(&ksyms_cv, &ksyms_lock);
+
+ /*
+ * Remove the symtab. Do this at splhigh to ensure ddb never
+ * witnesses an inconsistent state of the queue, unless memory
+ * is so corrupt that we crash in TAILQ_REMOVE.
+ */
+ s = splhigh();
+ TAILQ_REMOVE(&ksyms_symtabs, st, sd_queue);
+ splx(s);
+
+ /* Recompute the ksyms sizes now that we've removed st. */
+ ksyms_sizes_calc();
+ mutex_exit(&ksyms_lock);
+
+ /*
+ * No more references are possible. Free the name map and the
+ * symtab itself, which we had allocated in ksyms_modload.
+ */
+ kmem_free(st->sd_nmap, st->sd_nmapsize * sizeof(uint32_t));
+ kmem_free(st, sizeof(*st));
}
#ifdef DDB
@@ -830,8 +830,6 @@ ksyms_sift(char *mod, char *sym, int mod
return ENOENT;
TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) {
- if (st->sd_gone)
- continue;
if (mod && strcmp(mod, st->sd_name))
continue;
sb = st->sd_strstart - st->sd_usroffset;
@@ -893,8 +891,6 @@ ksyms_sizes_calc(void)
ksyms_symsz = ksyms_strsz = 0;
TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) {
- if (__predict_false(st->sd_gone))
- continue;
delta = ksyms_strsz - st->sd_usroffset;
if (delta != 0) {
for (i = 0; i < st->sd_symsize/sizeof(Elf_Sym); i++)
@@ -1027,37 +1023,14 @@ out: mutex_exit(&ksyms_lock);
static int
ksymsclose(dev_t dev, int oflags, int devtype, struct lwp *l)
{
- struct ksyms_symtab *st, *next;
- TAILQ_HEAD(, ksyms_symtab) to_free = TAILQ_HEAD_INITIALIZER(to_free);
- int s;
- /* Discard references to symbol tables. */
mutex_enter(&ksyms_lock);
if (--ksyms_opencnt)
goto out;
ksyms_last_snapshot = NULL;
- TAILQ_FOREACH_SAFE(st, &ksyms_symtabs, sd_queue, next) {
- if (st->sd_gone) {
- /*
- * Ensure ddb never witnesses an inconsistent
- * state of the queue, unless memory is so
- * corrupt that we crash in TAILQ_REMOVE.
- */
- s = splhigh();
- TAILQ_REMOVE(&ksyms_symtabs, st, sd_queue);
- splx(s);
- TAILQ_INSERT_TAIL(&to_free, st, sd_queue);
- }
- }
- if (!TAILQ_EMPTY(&to_free))
- ksyms_sizes_calc();
+ cv_broadcast(&ksyms_cv);
out: mutex_exit(&ksyms_lock);
- TAILQ_FOREACH_SAFE(st, &to_free, sd_queue, next) {
- kmem_free(st->sd_nmap, st->sd_nmapsize * sizeof(uint32_t));
- kmem_free(st, sizeof(*st));
- }
-
return 0;
}
@@ -1069,8 +1042,7 @@ ksymsread(dev_t dev, struct uio *uio, in
int error;
/*
- * First: Copy out the ELF header. XXX Lose if ksymsopen()
- * occurs during read of the header.
+ * First: Copy out the ELF header.
*/
off = uio->uio_offset;
if (off < sizeof(struct ksyms_hdr)) {
@@ -1081,12 +1053,17 @@ ksymsread(dev_t dev, struct uio *uio, in
}
/*
- * Copy out the symbol table.
+ * Copy out the symbol table. The list of symtabs is
+ * guaranteed to be nonempty because we always have an entry
+ * for the main kernel. We stop at ksyms_last_snapshot, not at
+ * the end of the tailq or NULL, because entries beyond
+ * ksyms_last_snapshot are not included in this snapshot (and
+ * may not be fully initialized memory as we witness it).
*/
filepos = sizeof(struct ksyms_hdr);
- TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) {
- if (__predict_false(st->sd_gone))
- continue;
+ for (st = TAILQ_FIRST(&ksyms_symtabs);
+ ;
+ st = TAILQ_NEXT(st, sd_queue)) {
if (uio->uio_resid == 0)
return 0;
if (uio->uio_offset <= st->sd_symsize + filepos) {
@@ -1125,6 +1102,8 @@ ksymsread(dev_t dev, struct uio *uio, in
/*
* Copy out the CTF table.
+ *
+ * XXX Why does this only copy out the first symtab's CTF?
*/
st = TAILQ_FIRST(&ksyms_symtabs);
if (st->sd_ctfstart != NULL) {
@@ -1196,8 +1175,6 @@ ksymsioctl(dev_t dev, u_long cmd, void *
*/
mutex_enter(&ksyms_lock);
TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) {
- if (st->sd_gone)
- continue;
if ((sym = findsym(str, st, KSYMS_ANY)) == NULL)
continue;
#ifdef notdef
@@ -1238,8 +1215,6 @@ ksymsioctl(dev_t dev, u_long cmd, void *
*/
mutex_enter(&ksyms_lock);
TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) {
- if (st->sd_gone)
- continue;
if ((sym = findsym(str, st, KSYMS_ANY)) == NULL)
continue;
#ifdef notdef
