Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 17 Jul 2003 22:11:05 -0700 (PDT)
From:      Matthew Dillon <dillon@apollo.backplane.com>
To:        hackers@freebsd.org
Subject:   Possible issue with in-kernel ELF loader.
Message-ID:  <200307180511.h6I5B5HV004858@apollo.backplane.com>

next in thread | raw e-mail | index | archive | help
   I got an odd bug report for DragonFly today.  Apparently the module loader
   was broken, the kernel complained about not being able to find
   'globaldata'.

   It turns out that the kernel's internal ELF loader is misinterpreting
   an ABS symbol (i.e. set with .SET in assembly) whos value is 0 as being
   'not found', and once that is fixed it was also confusing the absolute
   symbol with a COMMON symbol.

   While it appears neither -stable nor -current require the use of such
   a symbol in kernel modules, the ELF loader in the kernel probably ought
   to be fixed.  I've included the patch set I used on my tree below.  
   It should be fairly close to the 4.x tree.  Just FYI.

					-Matt
					Matthew Dillon 
					<dillon@backplane.com>

Index: i386/i386/elf_machdep.c
===================================================================
RCS file: /cvs/src/sys/i386/i386/elf_machdep.c,v
retrieving revision 1.2
diff -u -r1.2 elf_machdep.c
--- i386/i386/elf_machdep.c	17 Jun 2003 04:28:35 -0000	1.2
+++ i386/i386/elf_machdep.c	17 Jul 2003 23:21:47 -0000
@@ -40,6 +40,7 @@
 	Elf_Addr addr;
 	Elf_Addr addend;
 	Elf_Word rtype;
+	caddr_t caddr;
 	const Elf_Rel *rel;
 	const Elf_Rela *rela;
 
@@ -68,9 +69,9 @@
 		case R_386_32:		/* S + A */
 			if (sym == NULL)
 				return -1;
-			addr = (Elf_Addr)linker_file_lookup_symbol(lf, sym, 1);
-			if (addr == 0)
+			if (linker_file_lookup_symbol(lf, sym, 1, &caddr) != 0)
 				return -1;
+			addr = (Elf_Addr)caddr;
 			addr += addend;
 			if (*where != addr)
 				*where = addr;
@@ -79,9 +80,9 @@
 		case R_386_PC32:	/* S + A - P */
 			if (sym == NULL)
 				return -1;
-			addr = (Elf_Addr)linker_file_lookup_symbol(lf, sym, 1);
-			if (addr == 0)
+			if (linker_file_lookup_symbol(lf, sym, 1, &caddr) != 0)
 				return -1;
+			addr = (Elf_Addr)caddr;
 			addr += addend - (Elf_Addr)where;
 			if (*where != addr)
 				*where = addr;
@@ -99,9 +100,9 @@
 		case R_386_GLOB_DAT:	/* S */
 			if (sym == NULL)
 				return -1;
-			addr = (Elf_Addr)linker_file_lookup_symbol(lf, sym, 1);
-			if (addr == 0)
+			if (linker_file_lookup_symbol(lf, sym, 1, &caddr) != 0)
 				return -1;
+			addr = (Elf_Addr)caddr;
 			if (*where != addr)
 				*where = addr;
 			break;
Index: kern/kern_linker.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_linker.c,v
retrieving revision 1.6
diff -u -r1.6 kern_linker.c
--- kern/kern_linker.c	27 Jun 2003 01:53:25 -0000	1.6
+++ kern/kern_linker.c	17 Jul 2003 23:21:47 -0000
@@ -102,11 +102,12 @@
     KLD_DPF(FILE, ("linker_file_sysinit: calling SYSINITs for %s\n",
 		   lf->filename));
 
-    sysinits = (struct linker_set*)
-	linker_file_lookup_symbol(lf, "sysinit_set", 0);
-
+    if (linker_file_lookup_symbol(lf, "sysinit_set", 0, (caddr_t *)&sysinits)) {
+	KLD_DPF(FILE, ("linker_file_sysinit: SYSINITs not found\n"));
+	return 0; /* XXX is this correct ? No sysinit ? */
+    }
     KLD_DPF(FILE, ("linker_file_sysinit: SYSINITs %p\n", sysinits));
-    if (!sysinits)
+    if (sysinits == NULL)
 	return 0; /* XXX is this correct ? No sysinit ? */
 
     /* HACK ALERT! */
@@ -167,11 +168,12 @@
     KLD_DPF(FILE, ("linker_file_sysuninit: calling SYSUNINITs for %s\n",
 		   lf->filename));
 
-    sysuninits = (struct linker_set*)
-	linker_file_lookup_symbol(lf, "sysuninit_set", 0);
-
+    if (linker_file_lookup_symbol(lf, "sysuninit_set", 0, (caddr_t *)&sysuninits)) {
+	KLD_DPF(FILE, ("linker_file_sysuninit: SYSUNINITs not found\n"));
+	return;
+    }
     KLD_DPF(FILE, ("linker_file_sysuninit: SYSUNINITs %p\n", sysuninits));
-    if (!sysuninits)
+    if (sysuninits == NULL)
 	return;
 
     /*
@@ -215,11 +217,12 @@
     KLD_DPF(FILE, ("linker_file_register_sysctls: registering SYSCTLs for %s\n",
 		   lf->filename));
 
-    sysctls = (struct linker_set*)
-	linker_file_lookup_symbol(lf, "sysctl_set", 0);
-
+    if (linker_file_lookup_symbol(lf, "sysctl_set", 0, (caddr_t *)&sysctls)) {
+	KLD_DPF(FILE, ("linker_file_register_sysctls: SYSCTLs not found\n"));
+	return;
+    }
     KLD_DPF(FILE, ("linker_file_register_sysctls: SYSCTLs %p\n", sysctls));
-    if (!sysctls)
+    if (sysctls == NULL)
 	return;
 
     sysctl_register_set(sysctls);
@@ -233,11 +236,12 @@
     KLD_DPF(FILE, ("linker_file_unregister_sysctls: registering SYSCTLs for %s\n",
 		   lf->filename));
 
-    sysctls = (struct linker_set*)
-	linker_file_lookup_symbol(lf, "sysctl_set", 0);
-
+    if (linker_file_lookup_symbol(lf, "sysctl_set", 0, (caddr_t *)&sysctls)) {
+	KLD_DPF(FILE, ("linker_file_unregister_sysctls: SYSCTLs not found\n"));
+	return;
+    }
     KLD_DPF(FILE, ("linker_file_unregister_sysctls: SYSCTLs %p\n", sysctls));
-    if (!sysctls)
+    if (sysctls == NULL)
 	return;
 
     sysctl_unregister_set(sysctls);
@@ -478,13 +482,12 @@
     return 0;
 }
 
-caddr_t
-linker_file_lookup_symbol(linker_file_t file, const char* name, int deps)
+int
+linker_file_lookup_symbol(linker_file_t file, const char* name, int deps, caddr_t *raddr)
 {
     c_linker_sym_t sym;
     linker_symval_t symval;
     linker_file_t lf;
-    caddr_t address;
     size_t common_size = 0;
     int i;
 
@@ -493,24 +496,28 @@
 
     if (file->ops->lookup_symbol(file, name, &sym) == 0) {
 	file->ops->symbol_values(file, sym, &symval);
-	if (symval.value == 0)
+
+	/*
+	 * XXX Assume a common symbol if its value is 0 and it has a non-zero
+	 * size, otherwise it could be an absolute symbol with a value of 0.
+	 */
+	if (symval.value == 0 && symval.size != 0) {
 	    /*
 	     * For commons, first look them up in the dependancies and
 	     * only allocate space if not found there.
 	     */
 	    common_size = symval.size;
-	else {
+	} else {
 	    KLD_DPF(SYM, ("linker_file_lookup_symbol: symbol.value=%x\n", symval.value));
-	    return symval.value;
+	    *raddr = symval.value;
+	    return 0;
 	}
     }
-
     if (deps) {
 	for (i = 0; i < file->ndeps; i++) {
-	    address = linker_file_lookup_symbol(file->deps[i], name, 0);
-	    if (address) {
-		KLD_DPF(SYM, ("linker_file_lookup_symbol: deps value=%x\n", address));
-		return address;
+	    if (linker_file_lookup_symbol(file->deps[i], name, 0, raddr) == 0) {
+		KLD_DPF(SYM, ("linker_file_lookup_symbol: deps value=%x\n", *raddr));
+		return 0;
 	    }
 	}
 
@@ -525,10 +532,9 @@
 		    break;
 	    if (i < file->ndeps)
 		continue;
-	    address = linker_file_lookup_symbol(lf, name, 0);
-	    if (address) {
-		KLD_DPF(SYM, ("linker_file_lookup_symbol: global value=%x\n", address));
-		return address;
+	    if (linker_file_lookup_symbol(lf, name, 0, raddr) == 0) {
+		KLD_DPF(SYM, ("linker_file_lookup_symbol: global value=%x\n", *raddr));
+		return 0;
 	    }
 	}
     }
@@ -545,7 +551,8 @@
 	     cp = STAILQ_NEXT(cp, link))
 	    if (!strcmp(cp->name, name)) {
 		KLD_DPF(SYM, ("linker_file_lookup_symbol: old common value=%x\n", cp->address));
-		return cp->address;
+		*raddr = cp->address;
+		return 0;
 	    }
 
 	/*
@@ -558,7 +565,7 @@
 		    M_LINKER, M_WAITOK);
 	if (!cp) {
 	    KLD_DPF(SYM, ("linker_file_lookup_symbol: nomem\n"));
-	    return 0;
+	    return ENOMEM;
 	}
 	bzero(cp, sizeof(struct common_symbol) + common_size + strlen(name)+ 1);
 
@@ -569,11 +576,12 @@
 	STAILQ_INSERT_TAIL(&file->common, cp, link);
 
 	KLD_DPF(SYM, ("linker_file_lookup_symbol: new common value=%x\n", cp->address));
-	return cp->address;
+	*raddr = cp->address;
+	return 0;
     }
 
     KLD_DPF(SYM, ("linker_file_lookup_symbol: fail\n"));
-    return 0;
+    return ENOENT;
 }
 
 #ifdef DDB
@@ -947,9 +955,7 @@
 	if (lf) {
 	    lf->userrefs++;
 
-	    sysinits = (struct linker_set*)
-		linker_file_lookup_symbol(lf, "sysinit_set", 0);
-	    if (sysinits) {
+	    if (linker_file_lookup_symbol(lf, "sysinit_set", 0, (caddr_t *)&sysinits) == 0 && sysinits) {
 		/* HACK ALERT!
 		 * This is to set the sysinit moduledata so that the module
 		 * can attach itself to the correct containing file.
Index: kern/link_aout.c
===================================================================
RCS file: /cvs/src/sys/kern/link_aout.c,v
retrieving revision 1.4
diff -u -r1.4 link_aout.c
--- kern/link_aout.c	26 Jun 2003 05:55:14 -0000	1.4
+++ kern/link_aout.c	17 Jul 2003 23:21:47 -0000
@@ -408,14 +408,14 @@
 
 	    if (sym[0] != '_') {
 		printf("link_aout: bad symbol name %s\n", sym);
-		relocation = 0;
-	    } else
-		relocation = (intptr_t)
-		    linker_file_lookup_symbol(lf, sym + 1,
-					      np->nz_type != (N_SETV+N_EXT));
-	    if (!relocation) {
 		printf("link_aout: symbol %s not found\n", sym);
 		return ENOENT;
+	    } else {
+		if (linker_file_lookup_symbol(lf, sym + 1,
+		    (np->nz_type != (N_SETV+N_EXT)), (caddr_t *)&relocation)) {
+			printf("link_aout: symbol %s not found\n", sym);
+			return ENOENT;
+		}
 	    }
 	    
 	    relocation += read_relocation(r, addr);
Index: kern/link_elf.c
===================================================================
RCS file: /cvs/src/sys/kern/link_elf.c,v
retrieving revision 1.4
diff -u -r1.4 link_elf.c
--- kern/link_elf.c	26 Jun 2003 05:55:14 -0000	1.4
+++ kern/link_elf.c	17 Jul 2003 23:21:47 -0000
@@ -896,11 +896,13 @@
 	if (strcmp(name, strp) == 0) {
 	    if (symp->st_shndx != SHN_UNDEF ||
 		(symp->st_value != 0 &&
-		 ELF_ST_TYPE(symp->st_info) == STT_FUNC)) {
+		 ELF_ST_TYPE(symp->st_info) == STT_FUNC)
+	     ) {
 		*sym = (c_linker_sym_t) symp;
 		return 0;
-	    } else
+	    } else {
 		return ENOENT;
+	    }
 	}
 
 	symnum = ef->chains[symnum];
@@ -919,11 +921,11 @@
 		 ELF_ST_TYPE(symp->st_info) == STT_FUNC)) {
 		*sym = (c_linker_sym_t) symp;
 		return 0;
-	    } else
+	    } else {
 		return ENOENT;
+	    }
 	}
     }
-
     return ENOENT;
 }
 
Index: sys/linker.h
===================================================================
RCS file: /cvs/src/sys/sys/linker.h,v
retrieving revision 1.2
diff -u -r1.2 linker.h
--- sys/linker.h	17 Jun 2003 04:28:58 -0000	1.2
+++ sys/linker.h	17 Jul 2003 23:21:47 -0000
@@ -180,10 +180,11 @@
 
 /*
  * Lookup a symbol in a file.  If deps is TRUE, look in dependancies
- * if not found in file.
+ * if not found in file.  The symbol's value is returned in the
+ * caddr_t.  An error is returned, 0 if no error occured.
  */
-caddr_t linker_file_lookup_symbol(linker_file_t _file, const char* _name, 
-				  int _deps);
+int linker_file_lookup_symbol(linker_file_t _file, const char* _name, 
+				  int _deps, caddr_t *);
 
 /*
  * Search the linker path for the module.  Return the full pathname in



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200307180511.h6I5B5HV004858>