From 40b664f64b96112ea332157824502a0d5d88f015 Mon Sep 17 00:00:00 2001 From: Brandon Bergren Date: Sun, 21 Jun 2020 03:39:26 +0000 Subject: [PATCH] [PowerPC] More relocation fixes It turns out relocating the symbol table itself can cause issues, like fbt crashing because it applies the offsets to the kernel twice. This had been previously brought up in rS333447 when the stoffs hack was added, but I had been unaware of this and reimplemented symtab relocation. Instead of relocating the symbol table, keep track of the relocation base in ddb, so the ddb symbols behave like the kernel linker-provided symbols. This is intended to be NFC on platforms other than PowerPC, which do not use fully relocatable kernels. (The relbase will always be 0) * Remove the rest of the stoffs hack. * Remove my half-baked displace_symbol_table() function. * Extend ddb initialization to cope with having a relocation offset on the kernel symbol table. * Fix my kernel-as-initrd hack to work with booke64 by using a temporary mapping to access the data. * Fix another instance of __powerpc__ that is actually RELOCATABLE_KERNEL. * Change the behavior or X_db_symbol_values to apply the relocation base when updating valp, to match link_elf_symbol_values() behavior. Reviewed by: jhibbits Sponsored by: Tag1 Consulting, Inc. Differential Revision: https://reviews.freebsd.org/D25223 --- sys/amd64/amd64/machdep.c | 2 +- sys/arm/arm/machdep_boot.c | 2 +- sys/arm64/arm64/machdep_boot.c | 2 +- sys/ddb/db_main.c | 35 ++++++++--- sys/ddb/ddb.h | 9 +-- sys/dev/ksyms/ksyms.c | 2 +- sys/i386/i386/machdep.c | 2 +- sys/kern/link_elf.c | 16 +++++ sys/mips/mips/machdep.c | 2 +- sys/powerpc/powerpc/machdep.c | 103 ++++++++++++++++----------------- 10 files changed, 102 insertions(+), 73 deletions(-) diff --git a/sys/amd64/amd64/machdep.c b/sys/amd64/amd64/machdep.c index b2944089e6d9..6227344569df 100644 --- a/sys/amd64/amd64/machdep.c +++ b/sys/amd64/amd64/machdep.c @@ -1508,7 +1508,7 @@ native_parse_preload_data(u_int64_t modulep) #ifdef DDB ksym_start = MD_FETCH(kmdp, MODINFOMD_SSYM, uintptr_t); ksym_end = MD_FETCH(kmdp, MODINFOMD_ESYM, uintptr_t); - db_fetch_ksymtab(ksym_start, ksym_end); + db_fetch_ksymtab(ksym_start, ksym_end, 0); #endif efi_systbl_phys = MD_FETCH(kmdp, MODINFOMD_FW_HANDLE, vm_paddr_t); diff --git a/sys/arm/arm/machdep_boot.c b/sys/arm/arm/machdep_boot.c index 02ae321b623d..1b1b5c77b1dd 100644 --- a/sys/arm/arm/machdep_boot.c +++ b/sys/arm/arm/machdep_boot.c @@ -302,7 +302,7 @@ freebsd_parse_boot_param(struct arm_boot_params *abp) #ifdef DDB ksym_start = MD_FETCH(kmdp, MODINFOMD_SSYM, uintptr_t); ksym_end = MD_FETCH(kmdp, MODINFOMD_ESYM, uintptr_t); - db_fetch_ksymtab(ksym_start, ksym_end); + db_fetch_ksymtab(ksym_start, ksym_end, 0); #endif return lastaddr; } diff --git a/sys/arm64/arm64/machdep_boot.c b/sys/arm64/arm64/machdep_boot.c index 1f3fdd92e63d..b17cb3f7e2e1 100644 --- a/sys/arm64/arm64/machdep_boot.c +++ b/sys/arm64/arm64/machdep_boot.c @@ -208,7 +208,7 @@ freebsd_parse_boot_param(struct arm64_bootparams *abp) #ifdef DDB ksym_start = MD_FETCH(kmdp, MODINFOMD_SSYM, uintptr_t); ksym_end = MD_FETCH(kmdp, MODINFOMD_ESYM, uintptr_t); - db_fetch_ksymtab(ksym_start, ksym_end); + db_fetch_ksymtab(ksym_start, ksym_end, 0); #endif return (lastaddr); } diff --git a/sys/ddb/db_main.c b/sys/ddb/db_main.c index 885d531ad3dc..a52e6b2c3c95 100644 --- a/sys/ddb/db_main.c +++ b/sys/ddb/db_main.c @@ -48,6 +48,14 @@ __FBSDID("$FreeBSD$"); #include #include +struct db_private { + char* strtab; + vm_offset_t relbase; +}; +typedef struct db_private *db_private_t; + +#define DB_PRIVATE(x) ((db_private_t)(x->private)) + SYSCTL_NODE(_debug, OID_AUTO, ddb, CTLFLAG_RW | CTLFLAG_MPSAFE, 0, "DDB settings"); @@ -64,7 +72,8 @@ KDB_BACKEND(ddb, db_init, db_trace_self_wrapper, db_trace_thread_wrapper, * the symtab and strtab in memory. This is used when loaded from * boot loaders different than the native one (like Xen). */ -vm_offset_t ksymtab, kstrtab, ksymtab_size; +vm_offset_t ksymtab, kstrtab, ksymtab_size, ksymtab_relbase; +static struct db_private ksymtab_private; bool X_db_line_at_pc(db_symtab_t *symtab, c_db_sym_t sym, char **file, int *line, @@ -86,7 +95,8 @@ X_db_lookup(db_symtab_t *symtab, const char *symbol) sym = (Elf_Sym *)symtab->start; while ((char *)sym < symtab->end) { if (sym->st_name != 0 && - !strcmp(symtab->private + sym->st_name, symbol)) + !strcmp(DB_PRIVATE(symtab)->strtab + + sym->st_name, symbol)) return ((c_db_sym_t)sym); sym++; } @@ -101,7 +111,7 @@ X_db_search_symbol(db_symtab_t *symtab, db_addr_t off, db_strategy_t strat, c_linker_sym_t lsym; Elf_Sym *sym, *match; unsigned long diff; - db_addr_t stoffs; + db_addr_t stoffs = off; if (symtab->private == NULL) { if (!linker_ddb_search_symbol((caddr_t)off, &lsym, &diff)) { @@ -110,10 +120,11 @@ X_db_search_symbol(db_symtab_t *symtab, db_addr_t off, db_strategy_t strat, } return (NULL); } + else + stoffs -= DB_PRIVATE(symtab)->relbase; diff = ~0UL; match = NULL; - stoffs = DB_STOFFS(off); for (sym = (Elf_Sym*)symtab->start; (char*)sym < symtab->end; sym++) { if (sym->st_name == 0 || sym->st_shndx == SHN_UNDEF) continue; @@ -171,15 +182,17 @@ X_db_symbol_values(db_symtab_t *symtab, c_db_sym_t sym, const char **namep, *valp = (db_expr_t)lval.value; } else { if (namep != NULL) - *namep = (const char *)symtab->private + + *namep = (const char *)DB_PRIVATE(symtab)->strtab + ((const Elf_Sym *)sym)->st_name; if (valp != NULL) - *valp = (db_expr_t)((const Elf_Sym *)sym)->st_value; + *valp = (db_expr_t)((const Elf_Sym *)sym)->st_value + + DB_PRIVATE(symtab)->relbase; } } int -db_fetch_ksymtab(vm_offset_t ksym_start, vm_offset_t ksym_end) +db_fetch_ksymtab(vm_offset_t ksym_start, vm_offset_t ksym_end, + vm_offset_t relbase) { Elf_Size strsz; @@ -190,9 +203,11 @@ db_fetch_ksymtab(vm_offset_t ksym_start, vm_offset_t ksym_end) kstrtab = ksymtab + ksymtab_size; strsz = *(Elf_Size*)kstrtab; kstrtab += sizeof(Elf_Size); + ksymtab_relbase = relbase; if (kstrtab + strsz > ksym_end) { /* Sizes doesn't match, unset everything. */ - ksymtab = ksymtab_size = kstrtab = 0; + ksymtab = ksymtab_size = kstrtab = ksymtab_relbase + = 0; } } @@ -209,8 +224,10 @@ db_init(void) db_command_init(); if (ksymtab != 0 && kstrtab != 0 && ksymtab_size != 0) { + ksymtab_private.strtab = (char *)kstrtab; + ksymtab_private.relbase = ksymtab_relbase; db_add_symbol_table((char *)ksymtab, - (char *)(ksymtab + ksymtab_size), "elf", (char *)kstrtab); + (char *)(ksymtab + ksymtab_size), "elf", (char *)&ksymtab_private); } db_add_symbol_table(NULL, NULL, "kld", NULL); return (1); /* We're the default debugger. */ diff --git a/sys/ddb/ddb.h b/sys/ddb/ddb.h index d9452d4b5692..81448474c514 100644 --- a/sys/ddb/ddb.h +++ b/sys/ddb/ddb.h @@ -72,10 +72,6 @@ SYSCTL_DECL(_debug_ddb); #define DB_MAXSCRIPTRECURSION 3 #endif -#ifndef DB_STOFFS -#define DB_STOFFS(offs) (offs) -#endif - #ifndef DB_CALL #define DB_CALL db_fncall_generic #else @@ -87,7 +83,7 @@ int DB_CALL(db_expr_t, db_expr_t *, int, db_expr_t[]); * Most users should use db_fetch_symtab in order to set them from the * boot loader provided values. */ -extern vm_offset_t ksymtab, kstrtab, ksymtab_size; +extern vm_offset_t ksymtab, kstrtab, ksymtab_size, ksymtab_relbase; /* * There are three "command tables": @@ -232,7 +228,8 @@ bool db_value_of_name_vnet(const char *name, db_expr_t *valuep); int db_write_bytes(vm_offset_t addr, size_t size, char *data); void db_command_register(struct command_table *, struct command *); void db_command_unregister(struct command_table *, struct command *); -int db_fetch_ksymtab(vm_offset_t ksym_start, vm_offset_t ksym_end); +int db_fetch_ksymtab(vm_offset_t ksym_start, vm_offset_t ksym_end, + vm_offset_t relbase); db_cmdfcn_t db_breakpoint_cmd; db_cmdfcn_t db_capture_cmd; diff --git a/sys/dev/ksyms/ksyms.c b/sys/dev/ksyms/ksyms.c index 0f65b5b38d61..4a2752416763 100644 --- a/sys/dev/ksyms/ksyms.c +++ b/sys/dev/ksyms/ksyms.c @@ -202,7 +202,7 @@ ksyms_add(linker_file_t lf, void *arg) strsz = LINKER_STRTAB_GET(lf, &strtab); symsz = numsyms * sizeof(Elf_Sym); -#ifdef __powerpc__ +#ifdef RELOCATABLE_KERNEL fixup = true; #else fixup = lf->id > 1; diff --git a/sys/i386/i386/machdep.c b/sys/i386/i386/machdep.c index 6d66be311ab0..31561cad01f4 100644 --- a/sys/i386/i386/machdep.c +++ b/sys/i386/i386/machdep.c @@ -2180,7 +2180,7 @@ static void i386_kdb_init(void) { #ifdef DDB - db_fetch_ksymtab(bootinfo.bi_symtab, bootinfo.bi_esymtab); + db_fetch_ksymtab(bootinfo.bi_symtab, bootinfo.bi_esymtab, 0); #endif kdb_init(); #ifdef KDB diff --git a/sys/kern/link_elf.c b/sys/kern/link_elf.c index a3115b554d86..bf23e89a552e 100644 --- a/sys/kern/link_elf.c +++ b/sys/kern/link_elf.c @@ -389,6 +389,21 @@ link_elf_link_common_finish(linker_file_t lf) } #ifdef RELOCATABLE_KERNEL +/* + * __startkernel and __endkernel are symbols set up as relocation canaries. + * + * They are defined in locore to reference linker script symbols at the + * beginning and end of the LOAD area. This has the desired side effect of + * giving us variables that have relative relocations pointing at them, so + * relocation of the kernel object will cause the variables to be updated + * automatically by the runtime linker when we initialize. + * + * There are two main reasons to relocate the kernel: + * 1) If the loader needed to load the kernel at an alternate load address. + * 2) If the kernel is switching address spaces on machines like POWER9 + * under Radix where the high bits of the effective address are used to + * differentiate between hypervisor, host, guest, and problem state. + */ extern vm_offset_t __startkernel, __endkernel; #endif @@ -427,6 +442,7 @@ link_elf_init(void* arg) ef = (elf_file_t) linker_kernel_file; ef->preloaded = 1; #ifdef RELOCATABLE_KERNEL + /* Compute relative displacement */ ef->address = (caddr_t) (__startkernel - KERNBASE); #else ef->address = 0; diff --git a/sys/mips/mips/machdep.c b/sys/mips/mips/machdep.c index 909a3e2b907f..fd0f83e5df98 100644 --- a/sys/mips/mips/machdep.c +++ b/sys/mips/mips/machdep.c @@ -447,7 +447,7 @@ mips_postboot_fixup(void) kernel_kseg0_end += symtabsize; /* end of .strtab */ ksym_end = kernel_kseg0_end; - db_fetch_ksymtab(ksym_start, ksym_end); + db_fetch_ksymtab(ksym_start, ksym_end, 0); } #endif } diff --git a/sys/powerpc/powerpc/machdep.c b/sys/powerpc/powerpc/machdep.c index 0430773a9133..2bcc0ceed015 100644 --- a/sys/powerpc/powerpc/machdep.c +++ b/sys/powerpc/powerpc/machdep.c @@ -251,7 +251,6 @@ void booke_cpu_init(void); #ifdef DDB static void load_external_symtab(void); -static void displace_symbol_table(vm_offset_t, vm_offset_t, vm_offset_t); #endif uintptr_t @@ -360,16 +359,8 @@ powerpc_init(vm_offset_t fdt, vm_offset_t toc, vm_offset_t ofentry, void *mdp, ksym_end = MD_FETCH(kmdp, MODINFOMD_ESYM, uintptr_t); ksym_sz = *(Elf_Size*)ksym_start; - /* - * Loader already handled displacing to the load - * address, but we still need to displace it to the - * DMAP. - */ - displace_symbol_table( - (vm_offset_t)(ksym_start + sizeof(Elf_Size)), - ksym_sz, md_offset); - - db_fetch_ksymtab(ksym_start, ksym_end); + db_fetch_ksymtab(ksym_start, ksym_end, md_offset); + /* Symbols provided by loader. */ symbols_provided = true; #endif } @@ -509,37 +500,13 @@ powerpc_init(vm_offset_t fdt, vm_offset_t toc, vm_offset_t ofentry, void *mdp, #ifdef DDB /* - * XXX Figure out where to move this. - */ -static void -displace_symbol_table(vm_offset_t ksym_start, - vm_offset_t ksym_sz, vm_offset_t displacement) { - Elf_Sym *sym; - - /* - * Relocate the symbol table to our final load address. - */ - for (sym = (Elf_Sym *)ksym_start; - (vm_paddr_t)sym < (ksym_start + ksym_sz); - sym++) { - if (sym->st_name == 0 || - sym->st_shndx == SHN_UNDEF || - sym->st_value == 0) - continue; - if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT && - ELF_ST_TYPE(sym->st_info) != STT_FUNC && - ELF_ST_TYPE(sym->st_info) != STT_NOTYPE) - continue; - /* Skip relocating any implausible symbols */ - if (sym->st_value > KERNBASE) - sym->st_value += displacement; - } -} - -/* - * On powernv, we might not have symbols loaded via loader. However, if the - * user passed the kernel in as the initrd as well, we can manually load it - * via reinterpreting the initrd copy of the kernel. + * On powernv and some booke systems, we might not have symbols loaded via + * loader. However, if the user passed the kernel in as the initrd as well, + * we can manually load it via reinterpreting the initrd copy of the kernel. + * + * In the BOOKE case, we don't actually have a DMAP yet, so we have to use + * temporary maps to inspect the memory, but write DMAP addresses to the + * configuration variables. */ static void load_external_symtab(void) { @@ -547,7 +514,8 @@ load_external_symtab(void) { vm_paddr_t start, end; pcell_t cell[2]; ssize_t size; - u_char *kernelimg; + u_char *kernelimg; /* Temporary map */ + u_char *kernelimg_final; /* Final location */ int i; @@ -555,7 +523,8 @@ load_external_symtab(void) { Elf_Phdr *phdr; Elf_Shdr *shdr; - vm_offset_t ksym_start, ksym_sz, kstr_start, kstr_sz; + vm_offset_t ksym_start, ksym_sz, kstr_start, kstr_sz, + ksym_start_final, kstr_start_final; if (!hw_direct_map) return; @@ -587,27 +556,48 @@ load_external_symtab(void) { if (!(end - start > 0)) return; - kernelimg = (u_char *) PHYS_TO_DMAP(start); - + kernelimg_final = (u_char *) PHYS_TO_DMAP(start); +#ifdef AIM + kernelimg = kernelimg_final; +#else /* BOOKE */ + kernelimg = (u_char *)pmap_early_io_map(start, PAGE_SIZE); +#endif ehdr = (Elf_Ehdr *)kernelimg; - if (!IS_ELF(*ehdr)) + if (!IS_ELF(*ehdr)) { +#ifdef BOOKE + pmap_early_io_unmap(start, PAGE_SIZE); +#endif return; + } + +#ifdef BOOKE + pmap_early_io_unmap(start, PAGE_SIZE); + kernelimg = (u_char *)pmap_early_io_map(start, (end - start)); +#endif phdr = (Elf_Phdr *)(kernelimg + ehdr->e_phoff); shdr = (Elf_Shdr *)(kernelimg + ehdr->e_shoff); ksym_start = 0; ksym_sz = 0; + ksym_start_final = 0; kstr_start = 0; kstr_sz = 0; + kstr_start_final = 0; for (i = 0; i < ehdr->e_shnum; i++) { if (shdr[i].sh_type == SHT_SYMTAB) { ksym_start = (vm_offset_t)(kernelimg + shdr[i].sh_offset); + ksym_start_final = (vm_offset_t) + (kernelimg_final + shdr[i].sh_offset); ksym_sz = (vm_offset_t)(shdr[i].sh_size); kstr_start = (vm_offset_t)(kernelimg + shdr[shdr[i].sh_link].sh_offset); + kstr_start_final = (vm_offset_t) + (kernelimg_final + + shdr[shdr[i].sh_link].sh_offset); + kstr_sz = (vm_offset_t) (shdr[shdr[i].sh_link].sh_size); } @@ -615,14 +605,23 @@ load_external_symtab(void) { if (ksym_start != 0 && kstr_start != 0 && ksym_sz != 0 && kstr_sz != 0 && ksym_start < kstr_start) { - - displace_symbol_table(ksym_start, ksym_sz, - (__startkernel - KERNBASE)); - ksymtab = ksym_start; + /* + * We can't use db_fetch_ksymtab() here, because we need to + * feed in DMAP addresses that are not mapped yet on booke. + * + * Write the variables directly, where db_init() will pick + * them up later, after the DMAP is up. + */ + ksymtab = ksym_start_final; ksymtab_size = ksym_sz; - kstrtab = kstr_start; + kstrtab = kstr_start_final; + ksymtab_relbase = (__startkernel - KERNBASE); } +#ifdef BOOKE + pmap_early_io_unmap(start, (end - start)); +#endif + }; #endif