Module Name: src
Committed By: riastradh
Date: Tue Jun 1 21:10:23 UTC 2021
Modified Files:
src/sys/kern: kern_ksyms.c
Log Message:
ksyms(4): Fix ksymsread synchronization.
Fixes crash on concurrent update and read of /dev/ksyms.
XXX Unclear why we have to skip sd_gone entries here -- it seems like
they should be preserved until ksymsclose.
To generate a diff of this commit:
cvs rdiff -u -r1.89 -r1.90 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.89 src/sys/kern/kern_ksyms.c:1.90
--- src/sys/kern/kern_ksyms.c:1.89 Wed Sep 23 09:52:02 2020
+++ src/sys/kern/kern_ksyms.c Tue Jun 1 21:10:23 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_ksyms.c,v 1.89 2020/09/23 09:52:02 simonb Exp $ */
+/* $NetBSD: kern_ksyms.c,v 1.90 2021/06/01 21:10:23 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.89 2020/09/23 09:52:02 simonb Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_ksyms.c,v 1.90 2021/06/01 21:10:23 riastradh Exp $");
#if defined(_KERNEL) && defined(_KERNEL_OPT)
#include "opt_copy_symtab.h"
@@ -92,6 +92,7 @@ __KERNEL_RCSID(0, "$NetBSD: kern_ksyms.c
#include <sys/proc.h>
#include <sys/atomic.h>
#include <sys/ksyms.h>
+#include <sys/kernel.h>
#ifdef DDB
#include <ddb/db_output.h>
@@ -111,6 +112,7 @@ static uint32_t *ksyms_nmap = NULL;
static int ksyms_maxlen;
static bool ksyms_isopen;
+static struct ksyms_symtab *ksyms_last_snapshot;
static bool ksyms_initted;
static bool ksyms_loaded;
static kmutex_t ksyms_lock __cacheline_aligned;
@@ -140,7 +142,7 @@ struct ksyms_hdr ksyms_hdr;
int ksyms_symsz;
int ksyms_strsz;
int ksyms_ctfsz; /* this is not currently used by savecore(8) */
-TAILQ_HEAD(, ksyms_symtab) ksyms_symtabs =
+TAILQ_HEAD(ksyms_symtab_queue, ksyms_symtab) ksyms_symtabs =
TAILQ_HEAD_INITIALIZER(ksyms_symtabs);
static int
@@ -429,7 +431,7 @@ addsymtab(const char *name, void *symsta
for (new = 0; new < n; new++) {
uint32_t orig = nsym[new].st_size - 1;
uint32_t size = nmap[orig];
-
+
nmap[orig] = new + 1;
/* restore the size */
@@ -438,9 +440,11 @@ addsymtab(const char *name, void *symsta
}
#endif
- /* ksymsread() is unlocked, so membar. */
- membar_producer();
+ KASSERT(strcmp(name, "netbsd") == 0 || mutex_owned(&ksyms_lock));
+ KASSERT(cold || mutex_owned(&ksyms_lock));
+
TAILQ_INSERT_TAIL(&ksyms_symtabs, tab, sd_queue);
+
ksyms_sizes_calc();
ksyms_loaded = true;
}
@@ -767,6 +771,7 @@ void
ksyms_modunload(const char *name)
{
struct ksyms_symtab *st;
+ bool do_free = false;
mutex_enter(&ksyms_lock);
TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) {
@@ -778,14 +783,17 @@ ksyms_modunload(const char *name)
ksyms_sizes_calc();
if (!ksyms_isopen) {
TAILQ_REMOVE(&ksyms_symtabs, st, sd_queue);
- kmem_free(st->sd_nmap,
- st->sd_nmapsize * sizeof(uint32_t));
- kmem_free(st, sizeof(*st));
+ 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));
+ }
}
#ifdef DDB
@@ -864,6 +872,8 @@ ksyms_sizes_calc(void)
struct ksyms_symtab *st;
int i, delta;
+ KASSERT(cold || mutex_owned(&ksyms_lock));
+
ksyms_symsz = ksyms_strsz = 0;
TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) {
if (__predict_false(st->sd_gone))
@@ -981,6 +991,10 @@ ksymsopen(dev_t dev, int oflags, int dev
* ksyms_isopen will prevent symbol tables from being freed.
*/
mutex_enter(&ksyms_lock);
+ if (ksyms_isopen) {
+ mutex_exit(&ksyms_lock);
+ return EBUSY;
+ }
ksyms_hdr.kh_shdr[SYMTAB].sh_size = ksyms_symsz;
ksyms_hdr.kh_shdr[SYMTAB].sh_info = ksyms_symsz / sizeof(Elf_Sym);
ksyms_hdr.kh_shdr[STRTAB].sh_offset = ksyms_symsz +
@@ -990,6 +1004,7 @@ ksymsopen(dev_t dev, int oflags, int dev
ksyms_hdr.kh_shdr[STRTAB].sh_offset;
ksyms_hdr.kh_shdr[SHCTF].sh_size = ksyms_ctfsz;
ksyms_isopen = true;
+ ksyms_last_snapshot = TAILQ_LAST(&ksyms_symtabs, ksyms_symtab_queue);
mutex_exit(&ksyms_lock);
return 0;
@@ -999,26 +1014,27 @@ static int
ksymsclose(dev_t dev, int oflags, int devtype, struct lwp *l)
{
struct ksyms_symtab *st, *next;
- bool resize;
+ TAILQ_HEAD(, ksyms_symtab) to_free = TAILQ_HEAD_INITIALIZER(to_free);
/* Discard references to symbol tables. */
mutex_enter(&ksyms_lock);
ksyms_isopen = false;
- resize = false;
- for (st = TAILQ_FIRST(&ksyms_symtabs); st != NULL; st = next) {
- next = TAILQ_NEXT(st, sd_queue);
+ ksyms_last_snapshot = NULL;
+ TAILQ_FOREACH_SAFE(st, &ksyms_symtabs, sd_queue, next) {
if (st->sd_gone) {
TAILQ_REMOVE(&ksyms_symtabs, st, sd_queue);
- kmem_free(st->sd_nmap,
- st->sd_nmapsize * sizeof(uint32_t));
- kmem_free(st, sizeof(*st));
- resize = true;
+ TAILQ_INSERT_TAIL(&to_free, st, sd_queue);
}
}
- if (resize)
+ if (!TAILQ_EMPTY(&to_free))
ksyms_sizes_calc();
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;
}
@@ -1045,7 +1061,9 @@ ksymsread(dev_t dev, struct uio *uio, in
* Copy out the symbol table.
*/
filepos = sizeof(struct ksyms_hdr);
- TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) {
+ for (st = TAILQ_FIRST(&ksyms_symtabs);
+ st != ksyms_last_snapshot;
+ st = TAILQ_NEXT(st, sd_queue)) {
if (__predict_false(st->sd_gone))
continue;
if (uio->uio_resid == 0)
@@ -1063,9 +1081,11 @@ ksymsread(dev_t dev, struct uio *uio, in
/*
* Copy out the string table
*/
- KASSERT(filepos == sizeof(struct ksyms_hdr) +
+ KASSERT(filepos <= sizeof(struct ksyms_hdr) +
ksyms_hdr.kh_shdr[SYMTAB].sh_size);
- TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) {
+ for (st = TAILQ_FIRST(&ksyms_symtabs);
+ st != ksyms_last_snapshot;
+ st = TAILQ_NEXT(st, sd_queue)) {
if (__predict_false(st->sd_gone))
continue;
if (uio->uio_resid == 0)
