>Number:         21526
>Category:       kern
>Synopsis:       Bugs in ksyms
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun May 11 01:08:00 UTC 2003
>Closed-Date:
>Last-Modified:
>Originator:     Quentin Garnier
>Release:        NetBSD 1.6S
>Organization:
Quentin Garnier - cube@cubidou.net
"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.
>Environment:
	NetBSD 1.6S from today (well, before Jason's bump).
>Description:
	While working on a LKM, I found two bugs in ksyms:

	o Resolution of symbols from the LKM failed, possibly with a
	  segfault. This only happens when code is compiled for
	  /dev/ksyms support. After loading a symbol table,
	  ksyms_addsymtab() calls ksyms_sizes_calc() to adapt st_name
	  fields in the symbol table so that /dev/ksyms can present a
	  single STRTAB section to userland. ksyms_sizes_calc() adds
	  the necessary offset to each st_name value to do so, which
	  breaks the real meaning of the st_name field.

	o Symbols are not loaded into the kernel symbol table until
	  the entry function of the LKM returned successfully. This is
	  because it is up to the LKM to set its name in the lkm_table
	  struct. Then symbols are added, the name of the table being
	  the name of the module. This is quite annoying since it makes
	  very difficult to debug the entry function of the LKM.
>How-To-Repeat:
	Load a LKM, enter DDB and try resolving its symbols, with 'print'
	or 'sifting'.
>Fix:

	The very first hunk (one-liner) fixes the first bug. Note that it
	breaks the meaning of st->sd_strstart inside the kernel, but as
	long as it is always used along with the st_name field of a
	symbol, it is harmless.

	The rest adds code to allow access to the symbols of the LKM
	within DDB while the entry function of the LKM runs (well, panics).
	It simply loads the table with a generic name, and then rename it
	to its final name.

Index: kern/kern_ksyms.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_ksyms.c,v
retrieving revision 1.7
diff -u -r1.7 kern_ksyms.c
--- kern/kern_ksyms.c	2003/05/02 09:34:57	1.7
+++ kern/kern_ksyms.c	2003/05/11 00:37:47
@@ -369,10 +369,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_strstart -= strsz;
 		}
                 symsz += st->sd_symsize;
                 strsz += st->sd_strsize;
-        }                               
+        }
 }
 #endif
 
@@ -547,6 +548,31 @@
 	ksyms_sizes_calc();
 #endif
 	return 0;
+}
+
+int
+ksyms_rensymtab(char *old, 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(oldst->sd_name, M_DEVBUF);
+	oldst->sd_name = newstr;
+
+	return (0);
 }
 
 #ifdef DDB
Index: kern/kern_lkm.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_lkm.c,v
retrieving revision 1.65
diff -u -r1.65 kern_lkm.c
--- kern/kern_lkm.c	2003/04/24 20:09:43	1.65
+++ kern/kern_lkm.c	2003/05/11 00:37:48
@@ -410,11 +410,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, curp->ver);
+		(void)ksyms_rensymtab("/lkmtemp/", curp->private.lkm_any->lkm_name);
 		if (error) {
 			/*
 			 * Module may refuse loading or may have a
@@ -436,18 +450,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: sys/ksyms.h
===================================================================
RCS file: /cvsroot/src/sys/sys/ksyms.h,v
retrieving revision 1.4
diff -u -r1.4 ksyms.h
--- sys/ksyms.h	2003/05/01 20:46:20	1.4
+++ sys/ksyms.h	2003/05/11 00:37:48
@@ -63,6 +63,7 @@
 int ksyms_getval(char *, char *, unsigned long *, int);
 int ksyms_addsymtab(char *, void *, vsize_t, char *, vsize_t);
 int ksyms_delsymtab(char *);
+int ksyms_rensymtab(char *, char*);
 void ksyms_init(int, void *, void *);
 #ifdef DDB
 int ksyms_sift(char *mod, char *sym, int mode);
>Release-Note:
>Audit-Trail:
>Unformatted: