Subject: Bug in ksyms, kern/21526
To: None <tech-kern@netbsd.org>
From: Quentin Garnier <netbsd@quatriemek.com>
List: tech-kern
Date: 11/15/2003 19:12:06
This is a multi-part message in MIME format.
--Multipart_Sat__15_Nov_2003_19:12:06_+0100_08cf1e00
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Hi,
I would like to commit the attached patch, which actually solves the
problems outlined in kern/21256.
Currently, when support for /dev/ksyms is compiled in, ksyms_addsym will
shift every symbol value to make the symbol tables appear as one when
reading from /dev/ksyms. But this makes the value of the symbol invalid
when used in the kernel, and lead to crashes when loading a LKM module
that uses modules from other LKMs. The fact that the kernel's symbol
table appears as the first one means the kernel symbols values are not
shifted, otherwise loading any LKM would crash.
To fix that, the following patch adds a new parameter to findsym() and
most importantly to ksyms_getval() to indicate whether the request comes
from the kernel or the userland. It also adds to handy defines as
wrappers around the new ksyms_getval. Note that this change will require
a kernel version bump.
Also, there is currently no way of accessing the symbols from a LKM when
debugging its entry point. The reason for that is that the kernel expects
the LKM to set its own name, and thus wait for the completion of the entry
function of the LKM to load the symbol time. If the LKM crashes or panic
in that function, you can't access its symbols.
The attached patch introduces a new function ksyms_rensymtab that allows
renaming a symbol time. It is used when loading a LKM, by adding its
symbol table under the name "/lkmtemp/" before calling the entry function,
and renaming it to its final name afterwards.
I know that ksyms_addsymtab can sleep, so maybe I should not make the
temporary name unique, but rather add the pid of the modload process to
it. Also, maybe there should be some locking/sleeping in ksyms_rensymtab.
Comments? I know the LKM part is just a hack, but not by much more than
what we do to load LKMs, and a well designed in-kernel linker mechanism
will solve the issue. In the meantime, I think it is useful to be able to
access LKM symbols at that precise time.
--
Quentin Garnier - cube@cubidou.net - cube@NetBSD.org
"Feels like I'm fiddling while Rome is burning down.
Should I lay my fiddle down and take a rifle from the ground ?"
Leigh Nash/Sixpence None The Richer, Paralyzed, Divine Discontents, 2002.
--Multipart_Sat__15_Nov_2003_19:12:06_+0100_08cf1e00
Content-Type: text/plain;
name="ksyms.patch"
Content-Disposition: attachment;
filename="ksyms.patch"
Content-Transfer-Encoding: 7bit
Index: ddb/db_sym.c
===================================================================
RCS file: /cvsroot/src/sys/ddb/db_sym.c,v
retrieving revision 1.44
diff -u -r1.44 db_sym.c
--- ddb/db_sym.c 2003/10/25 08:54:01 1.44
+++ ddb/db_sym.c 2003/11/15 17:29:17
@@ -109,12 +109,12 @@
}
#endif
db_symsplit(name, &mod, &sym);
- if (ksyms_getval(mod, sym, &uval, KSYMS_EXTERN) == 0) {
+ if (ksyms_getval_from_kernel(mod, sym, &uval, KSYMS_EXTERN) == 0) {
val = (long) uval;
*valuep = (db_expr_t)val;
return TRUE;
}
- if (ksyms_getval(mod, sym, &uval, KSYMS_ANY) == 0) {
+ if (ksyms_getval_from_kernel(mod, sym, &uval, KSYMS_ANY) == 0) {
val = (long) uval;
*valuep = (db_expr_t)val;
return TRUE;
@@ -229,7 +229,7 @@
#endif
if (ksyms_getname(&mod, &sym, (vaddr_t)val, strategy) == 0) {
- (void)ksyms_getval(mod, sym, &naddr, KSYMS_ANY);
+ (void)ksyms_getval_from_kernel(mod, sym, &naddr, KSYMS_ANY);
diff = val - (db_addr_t)naddr;
ret = (db_sym_t)naddr;
} else
@@ -337,7 +337,7 @@
#endif
if (ksyms_getname(&mod, &name, (vaddr_t)off,
strategy|KSYMS_CLOSEST) == 0) {
- (void)ksyms_getval(mod, name, &val, KSYMS_ANY);
+ (void)ksyms_getval_from_kernel(mod, name, &val, KSYMS_ANY);
if (((off - val) < db_maxoff) && val) {
sprintf(buf, "%s:%s", mod, name);
if (off - val) {
@@ -407,7 +407,7 @@
#endif
if (ksyms_getname(&mod, &name, (vaddr_t)off,
strategy|KSYMS_CLOSEST) == 0) {
- (void)ksyms_getval(mod, name, &uval, KSYMS_ANY);
+ (void)ksyms_getval_from_kernel(mod, name, &uval, KSYMS_ANY);
val = (long) uval;
if (((off - val) < db_maxoff) && val) {
(*pr)("%s:%s", mod, name);
Index: kern/kern_lkm.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_lkm.c,v
retrieving revision 1.69
diff -u -r1.69 kern_lkm.c
--- kern/kern_lkm.c 2003/11/01 07:07:31 1.69
+++ kern/kern_lkm.c 2003/11/15 17:29:18
@@ -413,11 +413,25 @@
memset((caddr_t)curp->area + curp->offset, 0,
curp->size - curp->offset);
+ if (curp->syms && curp->sym_offset >= curp->sym_size) {
+ error = ksyms_addsymtab("/lkmtemp/",
+ (char *)curp->syms, curp->sym_symsize,
+ (char *)curp->syms + curp->sym_symsize,
+ curp->sym_size - curp->sym_symsize);
+ if (error)
+ break;
+#ifdef DEBUG
+ if (lkmdebug & LKMDB_INFO)
+ printf( "DDB symbols added!\n" );
+#endif
+ }
+
curp->entry = (int (*) __P((struct lkm_table *, int, int)))
(*((long *) (data)));
/* call entry(load)... (assigns "private" portion) */
error = (*(curp->entry))(curp, LKM_E_LOAD, LKM_VERSION);
+ (void)ksyms_rensymtab("/lkmtemp/", curp->private.lkm_any->lkm_name);
if (error) {
/*
* Module may refuse loading or may have a
@@ -439,18 +453,6 @@
if (lkmdebug & LKMDB_INFO)
printf("LKM: LMREADY\n");
#endif /* DEBUG */
- if (curp->syms && curp->sym_offset >= curp->sym_size) {
- error = ksyms_addsymtab(curp->private.lkm_any->lkm_name,
- (char *)curp->syms, curp->sym_symsize,
- (char *)curp->syms + curp->sym_symsize,
- curp->sym_size - curp->sym_symsize);
- if (error)
- break;
-#ifdef DEBUG
- if (lkmdebug & LKMDB_INFO)
- printf( "DDB symbols added!\n" );
-#endif
- }
lkm_state = LKMS_IDLE;
break;
Index: kern/kern_ksyms.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_ksyms.c,v
retrieving revision 1.16
diff -u -r1.16 kern_ksyms.c
--- kern/kern_ksyms.c 2003/11/06 18:22:01 1.16
+++ kern/kern_ksyms.c 2003/11/15 17:29:18
@@ -121,6 +121,7 @@
const char *sd_name; /* Name of this table */
Elf_Sym *sd_symstart; /* Address of symbol table */
caddr_t sd_strstart; /* Adderss of corresponding string table */
+ int sd_usroffset; /* Real address for userspace */
int sd_symsize; /* Size in bytes of symbol table */
int sd_strsize; /* Size of string table */
int *sd_symnmoff; /* Used when calculating the name offset */
@@ -268,11 +269,12 @@
* Finds a certain symbol name in a certain symbol table.
*/
static Elf_Sym *
-findsym(char *name, struct symtab *table)
+findsym(char *name, struct symtab *table, int userreq)
{
Elf_Sym *start = table->sd_symstart;
int i, sz = table->sd_symsize/sizeof(Elf_Sym);
char *np;
+ caddr_t realstart = table->sd_strstart - (userreq ? 0 : table->sd_usroffset);
#ifdef USE_PTREE
if (table == &kernel_symtab && (i = ptree_find(name)) != 0)
@@ -280,7 +282,7 @@
#endif
for (i = 0; i < sz; i++) {
- np = table->sd_strstart + start[i].st_name;
+ np = realstart + start[i].st_name;
if (name[0] == np[0] && name[1] == np[1] &&
strcmp(name, np) == 0)
return &start[i];
@@ -495,7 +497,7 @@
* Returns 0 if success or ENOENT if no such entry.
*/
int
-ksyms_getval(const char *mod, char *sym, unsigned long *val, int type)
+ksyms_getval(const char *mod, char *sym, unsigned long *val, int type, int userreq)
{
struct symtab *st;
Elf_Sym *es;
@@ -511,7 +513,7 @@
CIRCLEQ_FOREACH(st, &symtab_queue, sd_queue) {
if (mod && strcmp(st->sd_name, mod))
continue;
- if ((es = findsym(sym, st)) == NULL)
+ if ((es = findsym(sym, st, userreq)) == NULL)
continue;
/* Skip if bad binding */
@@ -563,7 +565,7 @@
laddr = les->st_value;
es = les;
lmod = st->sd_name;
- stable = st->sd_strstart;
+ stable = st->sd_strstart - st->sd_usroffset;
}
}
}
@@ -593,10 +595,11 @@
for (i = 0; i < st->sd_symsize/sizeof(Elf_Sym); i++)
st->sd_symstart[i].st_name =
strsz + st->sd_symnmoff[i];
+ st->sd_usroffset = strsz;
}
symsz += st->sd_symsize;
strsz += st->sd_strsize;
- }
+ }
}
#endif
@@ -693,7 +696,7 @@
continue;
/* Check if the symbol exists */
- if (ksyms_getval(NULL, strstart + sym[i].st_name,
+ if (ksyms_getval_from_kernel(NULL, strstart + sym[i].st_name,
&rval, KSYMS_EXTERN) == 0) {
/* Check (and complain) about differing values */
if (sym[i].st_value != rval) {
@@ -776,6 +779,31 @@
return 0;
}
+int
+ksyms_rensymtab(const char *old, const char *new)
+{
+ struct symtab *st, *oldst = NULL;
+ char *newstr;
+
+ CIRCLEQ_FOREACH(st, &symtab_queue, sd_queue) {
+ if (strcmp(old, st->sd_name) == 0)
+ oldst = st;
+ if (strcmp(new, st->sd_name) == 0)
+ return (EEXIST);
+ }
+ if (oldst == NULL)
+ return (ENOENT);
+
+ newstr = malloc(strlen(new)+1, M_DEVBUF, M_WAITOK);
+ if (!newstr)
+ return (ENOMEM);
+ strcpy(newstr, new);
+ free((char *)oldst->sd_name, M_DEVBUF);
+ oldst->sd_name = newstr;
+
+ return (0);
+}
+
#ifdef DDB
/*
@@ -801,7 +829,8 @@
Elf_Sym *les = st->sd_symstart + i;
char c;
- if (strstr(sb + les->st_name, sym) == NULL)
+ if (strstr(sb + les->st_name - st->sd_usroffset, sym)
+ == NULL)
continue;
if (mode == 'F') {
@@ -822,9 +851,11 @@
c = ' ';
break;
}
- db_printf("%s%c ", sb + les->st_name, c);
+ db_printf("%s%c ", sb + les->st_name -
+ st->sd_usroffset, c);
} else
- db_printf("%s ", sb + les->st_name);
+ db_printf("%s ", sb + les->st_name -
+ st->sd_usroffset);
}
}
return ENOENT;
@@ -1033,7 +1064,7 @@
*/
if ((error = copyinstr(kg->kg_name, str, ksyms_maxlen, NULL)))
break;
- if ((error = ksyms_getval(NULL, str, &val, KSYMS_EXTERN)))
+ if ((error = ksyms_getval_from_userland(NULL, str, &val, KSYMS_EXTERN)))
break;
error = copyout(&val, kg->kg_value, sizeof(long));
break;
@@ -1046,7 +1077,7 @@
if ((error = copyinstr(kg->kg_name, str, ksyms_maxlen, NULL)))
break;
CIRCLEQ_FOREACH(st, &symtab_queue, sd_queue) {
- if ((sym = findsym(str, st)) == NULL)
+ if ((sym = findsym(str, st, 1)) == NULL) /* from userland */
continue;
/* Skip if bad binding */
Index: sys/ksyms.h
===================================================================
RCS file: /cvsroot/src/sys/sys/ksyms.h,v
retrieving revision 1.7
diff -u -r1.7 ksyms.h
--- sys/ksyms.h 2003/07/08 06:32:15 1.7
+++ sys/ksyms.h 2003/11/15 17:29:18
@@ -60,9 +60,12 @@
* Prototypes
*/
int ksyms_getname(const char **, char **, vaddr_t, int);
-int ksyms_getval(const char *, char *, unsigned long *, int);
+int ksyms_getval(const char *, char *, unsigned long *, int, int);
+#define ksyms_getval_from_kernel(a,b,c,d) ksyms_getval(a,b,c,d,0)
+#define ksyms_getval_from_userland(a,b,c,d) ksyms_getval(a,b,c,d,1)
int ksyms_addsymtab(const char *, void *, vsize_t, char *, vsize_t);
int ksyms_delsymtab(const char *);
+int ksyms_rensymtab(const char *, const char*);
void ksyms_init(int, void *, void *);
#ifdef DDB
int ksyms_sift(char *, char *, int);
--Multipart_Sat__15_Nov_2003_19:12:06_+0100_08cf1e00--