Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 17 Sep 2015 00:43:21 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Gleb Smirnoff <glebius@FreeBSD.org>
Cc:        marcel@FreeBSD.org, Steve Kiernan <stevek@juniper.net>, brooks@FreeBSD.org, current@FreeBSD.org
Subject:   Re: r286727 breaks geom_md.ko loading
Message-ID:  <20150916214321.GI67105@kib.kiev.ua>
In-Reply-To: <20150916140411.GN1023@glebius.int.ru>
References:  <20150916000019.GH1023@FreeBSD.org> <20150916051005.GF67105@kib.kiev.ua> <20150916131515.GL1023@glebius.int.ru> <20150916134249.GG67105@kib.kiev.ua> <20150916135003.GM1023@glebius.int.ru> <20150916135739.GH67105@kib.kiev.ua> <20150916140411.GN1023@glebius.int.ru>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Sep 16, 2015 at 05:04:11PM +0300, Gleb Smirnoff wrote:
> Yes, sorry for not being verbose enough. Kernel must not have 'device md'
> and must have 'options MD_ROOT'.

The following worked for me on i386 and amd64.  Unless some objections are
raised against the approach, I will do the pass over other architectures
later.

diff --git a/sys/amd64/amd64/elf_machdep.c b/sys/amd64/amd64/elf_machdep.c
index d1cffd9..d961f09 100644
--- a/sys/amd64/amd64/elf_machdep.c
+++ b/sys/amd64/amd64/elf_machdep.c
@@ -168,6 +168,7 @@ elf_reloc_internal(linker_file_t lf, Elf_Addr relocbase, const void *data,
 	Elf_Size rtype, symidx;
 	const Elf_Rel *rel;
 	const Elf_Rela *rela;
+	int error;
 
 	switch (type) {
 	case ELF_RELOC_REL:
@@ -203,29 +204,29 @@ elf_reloc_internal(linker_file_t lf, Elf_Addr relocbase, const void *data,
 			break;
 
 		case R_X86_64_64:		/* S + A */
-			addr = lookup(lf, symidx, 1);
+			error = lookup(lf, symidx, 1, &addr);
 			val = addr + addend;
-			if (addr == 0)
+			if (error != 0)
 				return -1;
 			if (*where != val)
 				*where = val;
 			break;
 
 		case R_X86_64_PC32:	/* S + A - P */
-			addr = lookup(lf, symidx, 1);
+			error = lookup(lf, symidx, 1, &addr);
 			where32 = (Elf32_Addr *)where;
 			val32 = (Elf32_Addr)(addr + addend - (Elf_Addr)where);
-			if (addr == 0)
+			if (error != 0)
 				return -1;
 			if (*where32 != val32)
 				*where32 = val32;
 			break;
 
 		case R_X86_64_32S:	/* S + A sign extend */
-			addr = lookup(lf, symidx, 1);
+			error = lookup(lf, symidx, 1, &addr);
 			val32 = (Elf32_Addr)(addr + addend);
 			where32 = (Elf32_Addr *)where;
-			if (addr == 0)
+			if (error != 0)
 				return -1;
 			if (*where32 != val32)
 				*where32 = val32;
@@ -242,8 +243,8 @@ elf_reloc_internal(linker_file_t lf, Elf_Addr relocbase, const void *data,
 
 		case R_X86_64_GLOB_DAT:	/* S */
 		case R_X86_64_JMP_SLOT:	/* XXX need addend + offset */
-			addr = lookup(lf, symidx, 1);
-			if (addr == 0)
+			error = lookup(lf, symidx, 1, &addr);
+			if (error != 0)
 				return -1;
 			if (*where != addr)
 				*where = addr;
diff --git a/sys/i386/i386/elf_machdep.c b/sys/i386/i386/elf_machdep.c
index 03ab10f..81d6e35 100644
--- a/sys/i386/i386/elf_machdep.c
+++ b/sys/i386/i386/elf_machdep.c
@@ -178,6 +178,7 @@ elf_reloc_internal(linker_file_t lf, Elf_Addr relocbase, const void *data,
 	Elf_Word rtype, symidx;
 	const Elf_Rel *rel;
 	const Elf_Rela *rela;
+	int error;
 
 	switch (type) {
 	case ELF_RELOC_REL:
@@ -213,8 +214,8 @@ elf_reloc_internal(linker_file_t lf, Elf_Addr relocbase, const void *data,
 			break;
 
 		case R_386_32:		/* S + A */
-			addr = lookup(lf, symidx, 1);
-			if (addr == 0)
+			error = lookup(lf, symidx, 1, &addr);
+			if (error != 0)
 				return -1;
 			addr += addend;
 			if (*where != addr)
@@ -222,8 +223,8 @@ elf_reloc_internal(linker_file_t lf, Elf_Addr relocbase, const void *data,
 			break;
 
 		case R_386_PC32:	/* S + A - P */
-			addr = lookup(lf, symidx, 1);
-			if (addr == 0)
+			error = lookup(lf, symidx, 1, &addr);
+			if (error != 0)
 				return -1;
 			addr += addend - (Elf_Addr)where;
 			if (*where != addr)
@@ -240,8 +241,8 @@ elf_reloc_internal(linker_file_t lf, Elf_Addr relocbase, const void *data,
 			break;
 
 		case R_386_GLOB_DAT:	/* S */
-			addr = lookup(lf, symidx, 1);
-			if (addr == 0)
+			error = lookup(lf, symidx, 1, &addr);
+			if (error != 0)
 				return -1;
 			if (*where != addr)
 				*where = addr;
diff --git a/sys/kern/link_elf.c b/sys/kern/link_elf.c
index 3b741cc..c931ae9 100644
--- a/sys/kern/link_elf.c
+++ b/sys/kern/link_elf.c
@@ -158,7 +158,7 @@ static int	link_elf_each_function_nameval(linker_file_t,
 static void	link_elf_reloc_local(linker_file_t);
 static long	link_elf_symtab_get(linker_file_t, const Elf_Sym **);
 static long	link_elf_strtab_get(linker_file_t, caddr_t *);
-static Elf_Addr	elf_lookup(linker_file_t, Elf_Size, int);
+static int	elf_lookup(linker_file_t, Elf_Size, int, Elf_Addr *);
 
 static kobj_method_t link_elf_methods[] = {
 	KOBJMETHOD(linker_lookup_symbol,	link_elf_lookup_symbol),
@@ -1548,8 +1548,8 @@ elf_get_symname(linker_file_t lf, Elf_Size symidx)
  * This is not only more efficient, it's also more correct. It's not always
  * the case that the symbol can be found through the hash table.
  */
-static Elf_Addr
-elf_lookup(linker_file_t lf, Elf_Size symidx, int deps)
+static int
+elf_lookup(linker_file_t lf, Elf_Size symidx, int deps, Elf_Addr *res)
 {
 	elf_file_t ef = (elf_file_t)lf;
 	const Elf_Sym *sym;
@@ -1557,8 +1557,10 @@ elf_lookup(linker_file_t lf, Elf_Size symidx, int deps)
 	Elf_Addr addr, start, base;
 
 	/* Don't even try to lookup the symbol if the index is bogus. */
-	if (symidx >= ef->nchains)
-		return (0);
+	if (symidx >= ef->nchains) {
+		*res = 0;
+		return (EINVAL);
+	}
 
 	sym = ef->symtab + symidx;
 
@@ -1568,9 +1570,12 @@ elf_lookup(linker_file_t lf, Elf_Size symidx, int deps)
 	 */
 	if (ELF_ST_BIND(sym->st_info) == STB_LOCAL) {
 		/* Force lookup failure when we have an insanity. */
-		if (sym->st_shndx == SHN_UNDEF || sym->st_value == 0)
-			return (0);
-		return ((Elf_Addr)ef->address + sym->st_value);
+		if (sym->st_shndx == SHN_UNDEF || sym->st_value == 0) {
+			*res = 0;
+			return (EINVAL);
+		}
+		*res = ((Elf_Addr)ef->address + sym->st_value);
+		return (0);
 	}
 
 	/*
@@ -1583,8 +1588,10 @@ elf_lookup(linker_file_t lf, Elf_Size symidx, int deps)
 	symbol = ef->strtab + sym->st_name;
 
 	/* Force a lookup failure if the symbol name is bogus. */
-	if (*symbol == 0)
-		return (0);
+	if (*symbol == 0) {
+		*res = 0;
+		return (EINVAL);
+	}
 
 	addr = ((Elf_Addr)linker_file_lookup_symbol(lf, symbol, deps));
 
@@ -1594,7 +1601,8 @@ elf_lookup(linker_file_t lf, Elf_Size symidx, int deps)
 	else if (elf_set_find(&set_vnet_list, addr, &start, &base))
 		addr = addr - start + base;
 #endif
-	return addr;
+	*res = addr;
+	return (0);
 }
 
 static void
diff --git a/sys/kern/link_elf_obj.c b/sys/kern/link_elf_obj.c
index 021381d..00fe1e4 100644
--- a/sys/kern/link_elf_obj.c
+++ b/sys/kern/link_elf_obj.c
@@ -144,7 +144,8 @@ static void	link_elf_reloc_local(linker_file_t);
 static long	link_elf_symtab_get(linker_file_t, const Elf_Sym **);
 static long	link_elf_strtab_get(linker_file_t, caddr_t *);
 
-static Elf_Addr elf_obj_lookup(linker_file_t lf, Elf_Size symidx, int deps);
+static int	elf_obj_lookup(linker_file_t lf, Elf_Size symidx, int deps,
+		    Elf_Addr *);
 
 static kobj_method_t link_elf_methods[] = {
 	KOBJMETHOD(linker_lookup_symbol,	link_elf_lookup_symbol),
@@ -1253,38 +1254,46 @@ elf_obj_cleanup_globals_cache(elf_file_t ef)
  * This is not only more efficient, it's also more correct. It's not always
  * the case that the symbol can be found through the hash table.
  */
-static Elf_Addr
-elf_obj_lookup(linker_file_t lf, Elf_Size symidx, int deps)
+static int
+elf_obj_lookup(linker_file_t lf, Elf_Size symidx, int deps, Elf_Addr *res)
 {
 	elf_file_t ef = (elf_file_t)lf;
 	Elf_Sym *sym;
 	const char *symbol;
-	Elf_Addr ret;
+	Elf_Addr res1;
 
 	/* Don't even try to lookup the symbol if the index is bogus. */
-	if (symidx >= ef->ddbsymcnt)
-		return (0);
+	if (symidx >= ef->ddbsymcnt) {
+		*res = 0;
+		return (EINVAL);
+	}
 
 	sym = ef->ddbsymtab + symidx;
 
 	/* Quick answer if there is a definition included. */
-	if (sym->st_shndx != SHN_UNDEF)
-		return (sym->st_value);
+	if (sym->st_shndx != SHN_UNDEF) {
+		*res = sym->st_value;
+		return (0);
+	}
 
 	/* If we get here, then it is undefined and needs a lookup. */
 	switch (ELF_ST_BIND(sym->st_info)) {
 	case STB_LOCAL:
 		/* Local, but undefined? huh? */
-		return (0);
+		*res = 0;
+		return (EINVAL);
 
 	case STB_GLOBAL:
+	case STB_WEAK:
 		/* Relative to Data or Function name */
 		symbol = ef->ddbstrtab + sym->st_name;
 
 		/* Force a lookup failure if the symbol name is bogus. */
-		if (*symbol == 0)
-			return (0);
-		ret = ((Elf_Addr)linker_file_lookup_symbol(lf, symbol, deps));
+		if (*symbol == 0) {
+			*res = 0;
+			return (EINVAL);
+		}
+		res1 = (Elf_Addr)linker_file_lookup_symbol(lf, symbol, deps);
 
 		/*
 		 * Cache global lookups during module relocation. The failure
@@ -1296,18 +1305,20 @@ elf_obj_lookup(linker_file_t lf, Elf_Size symidx, int deps)
 		 * restored to SHN_UNDEF in elf_obj_cleanup_globals_cache(),
 		 * above.
 		 */
-		if (ret != 0) {
+		if (res1 != 0) {
 			sym->st_shndx = SHN_FBSD_CACHED;
-			sym->st_value = ret;
+			sym->st_value = res1;
+			*res = res1;
+			return (0);
+		} else if (ELF_ST_BIND(sym->st_info) == STB_WEAK) {
+			sym->st_value = 0;
+			*res = 0;
+			return (0);
 		}
-		return (ret);
-
-	case STB_WEAK:
-		printf("link_elf_obj: Weak symbols not supported\n");
-		return (0);
+		return (EINVAL);
 
 	default:
-		return (0);
+		return (EINVAL);
 	}
 }
 
diff --git a/sys/sys/linker.h b/sys/sys/linker.h
index 5d24f00..7e562a6 100644
--- a/sys/sys/linker.h
+++ b/sys/sys/linker.h
@@ -264,7 +264,7 @@ extern int kld_debug;
 
 #endif
 
-typedef Elf_Addr elf_lookup_fn(linker_file_t, Elf_Size, int);
+typedef int elf_lookup_fn(linker_file_t, Elf_Size, int, Elf_Addr *);
 
 /* Support functions */
 int	elf_reloc(linker_file_t _lf, Elf_Addr base, const void *_rel, int _type, elf_lookup_fn _lu);



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