From 4074532758c5c367d3fcb8d124150824a254659d Mon Sep 17 00:00:00 2001 From: Jack Brennen Date: Tue, 26 Sep 2023 08:40:44 -0400 Subject: modpost: Optimize symbol search from linear to binary search Modify modpost to use binary search for converting addresses back into symbol references. Previously it used linear search. This change saves a few seconds of wall time for defconfig builds, but can save several minutes on allyesconfigs. Before: $ make LLVM=1 -j128 allyesconfig vmlinux -s KCFLAGS="-Wno-error" $ time scripts/mod/modpost -M -m -a -N -o vmlinux.symvers vmlinux.o 198.38user 1.27system 3:19.71elapsed After: $ make LLVM=1 -j128 allyesconfig vmlinux -s KCFLAGS="-Wno-error" $ time scripts/mod/modpost -M -m -a -N -o vmlinux.symvers vmlinux.o 11.91user 0.85system 0:12.78elapsed Signed-off-by: Jack Brennen Tested-by: Nick Desaulniers Signed-off-by: Masahiro Yamada --- scripts/mod/Makefile | 4 +- scripts/mod/modpost.c | 70 ++--------------- scripts/mod/modpost.h | 25 ++++++ scripts/mod/symsearch.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 232 insertions(+), 66 deletions(-) create mode 100644 scripts/mod/symsearch.c (limited to 'scripts/mod') diff --git a/scripts/mod/Makefile b/scripts/mod/Makefile index c9e38ad937fd..3c54125eb373 100644 --- a/scripts/mod/Makefile +++ b/scripts/mod/Makefile @@ -5,7 +5,7 @@ CFLAGS_REMOVE_empty.o += $(CC_FLAGS_LTO) hostprogs-always-y += modpost mk_elfconfig always-y += empty.o -modpost-objs := modpost.o file2alias.o sumversion.o +modpost-objs := modpost.o file2alias.o sumversion.o symsearch.o devicetable-offsets-file := devicetable-offsets.h @@ -16,7 +16,7 @@ targets += $(devicetable-offsets-file) devicetable-offsets.s # dependencies on generated files need to be listed explicitly -$(obj)/modpost.o $(obj)/file2alias.o $(obj)/sumversion.o: $(obj)/elfconfig.h +$(obj)/modpost.o $(obj)/file2alias.o $(obj)/sumversion.o $(obj)/symsearch.o: $(obj)/elfconfig.h $(obj)/file2alias.o: $(obj)/$(devicetable-offsets-file) quiet_cmd_elfconfig = MKELF $@ diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index b3dee80497cb..2f3b0fe6f68d 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -22,7 +22,6 @@ #include #include "modpost.h" #include "../../include/linux/license.h" -#include "../../include/linux/module_symbol.h" static bool module_enabled; /* Are we using CONFIG_MODVERSIONS? */ @@ -577,11 +576,14 @@ static int parse_elf(struct elf_info *info, const char *filename) *p = TO_NATIVE(*p); } + symsearch_init(info); + return 1; } static void parse_elf_finish(struct elf_info *info) { + symsearch_finish(info); release_file(info->hdr, info->size); } @@ -1050,71 +1052,10 @@ static int secref_whitelist(const char *fromsec, const char *fromsym, return 1; } -/* - * If there's no name there, ignore it; likewise, ignore it if it's - * one of the magic symbols emitted used by current tools. - * - * Otherwise if find_symbols_between() returns those symbols, they'll - * fail the whitelist tests and cause lots of false alarms ... fixable - * only by merging __exit and __init sections into __text, bloating - * the kernel (which is especially evil on embedded platforms). - */ -static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym) -{ - const char *name = elf->strtab + sym->st_name; - - if (!name || !strlen(name)) - return 0; - return !is_mapping_symbol(name); -} - -/* Look up the nearest symbol based on the section and the address */ -static Elf_Sym *find_nearest_sym(struct elf_info *elf, Elf_Addr addr, - unsigned int secndx, bool allow_negative, - Elf_Addr min_distance) -{ - Elf_Sym *sym; - Elf_Sym *near = NULL; - Elf_Addr sym_addr, distance; - bool is_arm = (elf->hdr->e_machine == EM_ARM); - - for (sym = elf->symtab_start; sym < elf->symtab_stop; sym++) { - if (get_secindex(elf, sym) != secndx) - continue; - if (!is_valid_name(elf, sym)) - continue; - - sym_addr = sym->st_value; - - /* - * For ARM Thumb instruction, the bit 0 of st_value is set - * if the symbol is STT_FUNC type. Mask it to get the address. - */ - if (is_arm && ELF_ST_TYPE(sym->st_info) == STT_FUNC) - sym_addr &= ~1; - - if (addr >= sym_addr) - distance = addr - sym_addr; - else if (allow_negative) - distance = sym_addr - addr; - else - continue; - - if (distance <= min_distance) { - min_distance = distance; - near = sym; - } - - if (min_distance == 0) - break; - } - return near; -} - static Elf_Sym *find_fromsym(struct elf_info *elf, Elf_Addr addr, unsigned int secndx) { - return find_nearest_sym(elf, addr, secndx, false, ~0); + return symsearch_find_nearest(elf, addr, secndx, false, ~0); } static Elf_Sym *find_tosym(struct elf_info *elf, Elf_Addr addr, Elf_Sym *sym) @@ -1127,7 +1068,8 @@ static Elf_Sym *find_tosym(struct elf_info *elf, Elf_Addr addr, Elf_Sym *sym) * Strive to find a better symbol name, but the resulting name may not * match the symbol referenced in the original code. */ - return find_nearest_sym(elf, addr, get_secindex(elf, sym), true, 20); + return symsearch_find_nearest(elf, addr, get_secindex(elf, sym), + true, 20); } static bool is_executable_section(struct elf_info *elf, unsigned int secndx) diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h index 5f94c2c9f2d9..6413f26fcb6b 100644 --- a/scripts/mod/modpost.h +++ b/scripts/mod/modpost.h @@ -10,6 +10,7 @@ #include #include #include +#include "../../include/linux/module_symbol.h" #include "list.h" #include "elfconfig.h" @@ -128,6 +129,8 @@ struct elf_info { * take shndx from symtab_shndx_start[N] instead */ Elf32_Word *symtab_shndx_start; Elf32_Word *symtab_shndx_stop; + + struct symsearch *symsearch; }; /* Accessor for sym->st_shndx, hides ugliness of "64k sections" */ @@ -154,6 +157,28 @@ static inline unsigned int get_secindex(const struct elf_info *info, return index; } +/* + * If there's no name there, ignore it; likewise, ignore it if it's + * one of the magic symbols emitted used by current tools. + * + * Internal symbols created by tools should be ignored by modpost. + */ +static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym) +{ + const char *name = elf->strtab + sym->st_name; + + if (!name || !strlen(name)) + return 0; + return !is_mapping_symbol(name); +} + +/* symsearch.c */ +void symsearch_init(struct elf_info *elf); +void symsearch_finish(struct elf_info *elf); +Elf_Sym *symsearch_find_nearest(struct elf_info *elf, Elf_Addr addr, + unsigned int secndx, bool allow_negative, + Elf_Addr min_distance); + /* file2alias.c */ void handle_moddevtable(struct module *mod, struct elf_info *info, Elf_Sym *sym, const char *symname); diff --git a/scripts/mod/symsearch.c b/scripts/mod/symsearch.c new file mode 100644 index 000000000000..aa4ed51f9960 --- /dev/null +++ b/scripts/mod/symsearch.c @@ -0,0 +1,199 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * Helper functions for finding the symbol in an ELF which is "nearest" + * to a given address. + */ + +#include "modpost.h" + +struct syminfo { + unsigned int symbol_index; + unsigned int section_index; + Elf_Addr addr; +}; + +/* + * Container used to hold an entire binary search table. + * Entries in table are ascending, sorted first by section_index, + * then by addr, and last by symbol_index. The sorting by + * symbol_index is used to ensure predictable behavior when + * multiple symbols are present with the same address; all + * symbols past the first are effectively ignored, by eliding + * them in symsearch_fixup(). + */ +struct symsearch { + unsigned int table_size; + struct syminfo table[]; +}; + +static int syminfo_compare(const void *s1, const void *s2) +{ + const struct syminfo *sym1 = s1; + const struct syminfo *sym2 = s2; + + if (sym1->section_index > sym2->section_index) + return 1; + if (sym1->section_index < sym2->section_index) + return -1; + if (sym1->addr > sym2->addr) + return 1; + if (sym1->addr < sym2->addr) + return -1; + if (sym1->symbol_index > sym2->symbol_index) + return 1; + if (sym1->symbol_index < sym2->symbol_index) + return -1; + return 0; +} + +static unsigned int symbol_count(struct elf_info *elf) +{ + unsigned int result = 0; + + for (Elf_Sym *sym = elf->symtab_start; sym < elf->symtab_stop; sym++) { + if (is_valid_name(elf, sym)) + result++; + } + return result; +} + +/* + * Populate the search array that we just allocated. + * Be slightly paranoid here. The ELF file is mmap'd and could + * conceivably change between symbol_count() and symsearch_populate(). + * If we notice any difference, bail out rather than potentially + * propagating errors or crashing. + */ +static void symsearch_populate(struct elf_info *elf, + struct syminfo *table, + unsigned int table_size) +{ + bool is_arm = (elf->hdr->e_machine == EM_ARM); + + for (Elf_Sym *sym = elf->symtab_start; sym < elf->symtab_stop; sym++) { + if (is_valid_name(elf, sym)) { + if (table_size-- == 0) + fatal("%s: size mismatch\n", __func__); + table->symbol_index = sym - elf->symtab_start; + table->section_index = get_secindex(elf, sym); + table->addr = sym->st_value; + + /* + * For ARM Thumb instruction, the bit 0 of st_value is + * set if the symbol is STT_FUNC type. Mask it to get + * the address. + */ + if (is_arm && ELF_ST_TYPE(sym->st_info) == STT_FUNC) + table->addr &= ~1; + + table++; + } + } + + if (table_size != 0) + fatal("%s: size mismatch\n", __func__); +} + +/* + * Do any fixups on the table after sorting. + * For now, this just finds adjacent entries which have + * the same section_index and addr, and it propagates + * the first symbol_index over the subsequent entries, + * so that only one symbol_index is seen for any given + * section_index and addr. This ensures that whether + * we're looking at an address from "above" or "below" + * that we see the same symbol_index. + * This does leave some duplicate entries in the table; + * in practice, these are a small fraction of the + * total number of entries, and they are harmless to + * the binary search algorithm other than a few occasional + * unnecessary comparisons. + */ +static void symsearch_fixup(struct syminfo *table, unsigned int table_size) +{ + /* Don't look at index 0, it will never change. */ + for (unsigned int i = 1; i < table_size; i++) { + if (table[i].addr == table[i - 1].addr && + table[i].section_index == table[i - 1].section_index) { + table[i].symbol_index = table[i - 1].symbol_index; + } + } +} + +void symsearch_init(struct elf_info *elf) +{ + unsigned int table_size = symbol_count(elf); + + elf->symsearch = NOFAIL(malloc(sizeof(struct symsearch) + + sizeof(struct syminfo) * table_size)); + elf->symsearch->table_size = table_size; + + symsearch_populate(elf, elf->symsearch->table, table_size); + qsort(elf->symsearch->table, table_size, + sizeof(struct syminfo), syminfo_compare); + + symsearch_fixup(elf->symsearch->table, table_size); +} + +void symsearch_finish(struct elf_info *elf) +{ + free(elf->symsearch); + elf->symsearch = NULL; +} + +/* + * Find the syminfo which is in secndx and "nearest" to addr. + * allow_negative: allow returning a symbol whose address is > addr. + * min_distance: ignore symbols which are further away than this. + * + * Returns a pointer into the symbol table for success. + * Returns NULL if no legal symbol is found within the requested range. + */ +Elf_Sym *symsearch_find_nearest(struct elf_info *elf, Elf_Addr addr, + unsigned int secndx, bool allow_negative, + Elf_Addr min_distance) +{ + unsigned int hi = elf->symsearch->table_size; + unsigned int lo = 0; + struct syminfo *table = elf->symsearch->table; + struct syminfo target; + + target.addr = addr; + target.section_index = secndx; + target.symbol_index = ~0; /* compares greater than any actual index */ + while (hi > lo) { + unsigned int mid = lo + (hi - lo) / 2; /* Avoids overflow */ + + if (syminfo_compare(&table[mid], &target) > 0) + hi = mid; + else + lo = mid + 1; + } + + /* + * table[hi], if it exists, is the first entry in the array which + * lies beyond target. table[hi - 1], if it exists, is the last + * entry in the array which comes before target, including the + * case where it perfectly matches the section and the address. + * + * Note -- if the address we're looking up falls perfectly + * in the middle of two symbols, this is written to always + * prefer the symbol with the lower address. + */ + Elf_Sym *result = NULL; + + if (allow_negative && + hi < elf->symsearch->table_size && + table[hi].section_index == secndx && + table[hi].addr - addr <= min_distance) { + min_distance = table[hi].addr - addr; + result = &elf->symtab_start[table[hi].symbol_index]; + } + if (hi > 0 && + table[hi - 1].section_index == secndx && + addr - table[hi - 1].addr <= min_distance) { + result = &elf->symtab_start[table[hi - 1].symbol_index]; + } + return result; +} -- cgit From 7f54e00e5842663c2cea501bbbdfa572c94348a3 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Sun, 8 Oct 2023 02:04:44 +0900 Subject: modpost: fix tee MODULE_DEVICE_TABLE built on big-endian host When MODULE_DEVICE_TABLE(tee, ) is built on a host with a different endianness from the target architecture, it results in an incorrect MODULE_ALIAS(). For example, see a case where drivers/char/hw_random/optee-rng.c is built as a module for ARM little-endian. If you build it on a little-endian host, you will get the correct MODULE_ALIAS: $ grep MODULE_ALIAS drivers/char/hw_random/optee-rng.mod.c MODULE_ALIAS("tee:ab7a617c-b8e7-4d8f-8301-d09b61036b64*"); However, if you build it on a big-endian host, you will get a wrong MODULE_ALIAS: $ grep MODULE_ALIAS drivers/char/hw_random/optee-rng.mod.c MODULE_ALIAS("tee:646b0361-9bd0-0183-8f4d-e7b87c617aab*"); The same problem also occurs when you enable CONFIG_CPU_BIG_ENDIAN, and build it on a little-endian host. This issue has been unnoticed because the ARM kernel is configured for little-endian by default, and most likely built on a little-endian host (cross-build on x86 or native-build on ARM). The uuid field must not be reversed because uuid_t is an array of __u8. Fixes: 0fc1db9d1059 ("tee: add bus driver framework for TEE based devices") Signed-off-by: Masahiro Yamada Reviewed-by: Sumit Garg --- scripts/mod/file2alias.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'scripts/mod') diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c index 7056751c29b1..70bf6a2f585c 100644 --- a/scripts/mod/file2alias.c +++ b/scripts/mod/file2alias.c @@ -1348,13 +1348,13 @@ static int do_typec_entry(const char *filename, void *symval, char *alias) /* Looks like: tee:uuid */ static int do_tee_entry(const char *filename, void *symval, char *alias) { - DEF_FIELD(symval, tee_client_device_id, uuid); + DEF_FIELD_ADDR(symval, tee_client_device_id, uuid); sprintf(alias, "tee:%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x", - uuid.b[0], uuid.b[1], uuid.b[2], uuid.b[3], uuid.b[4], - uuid.b[5], uuid.b[6], uuid.b[7], uuid.b[8], uuid.b[9], - uuid.b[10], uuid.b[11], uuid.b[12], uuid.b[13], uuid.b[14], - uuid.b[15]); + uuid->b[0], uuid->b[1], uuid->b[2], uuid->b[3], uuid->b[4], + uuid->b[5], uuid->b[6], uuid->b[7], uuid->b[8], uuid->b[9], + uuid->b[10], uuid->b[11], uuid->b[12], uuid->b[13], uuid->b[14], + uuid->b[15]); add_wildcard(alias); return 1; -- cgit From ac96a15a0f0c8812a3aaa587b871cd5527f6d736 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Sun, 8 Oct 2023 02:04:45 +0900 Subject: modpost: fix ishtp MODULE_DEVICE_TABLE built on big-endian host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When MODULE_DEVICE_TABLE(ishtp, ) is built on a host with a different endianness from the target architecture, it results in an incorrect MODULE_ALIAS(). For example, see a case where drivers/platform/x86/intel/ishtp_eclite.c is built as a module for x86. If you build it on a little-endian host, you will get the correct MODULE_ALIAS: $ grep MODULE_ALIAS drivers/platform/x86/intel/ishtp_eclite.mod.c MODULE_ALIAS("ishtp:{6A19CC4B-D760-4DE3-B14D-F25EBD0FBCD9}"); However, if you build it on a big-endian host, you will get a wrong MODULE_ALIAS: $ grep MODULE_ALIAS drivers/platform/x86/intel/ishtp_eclite.mod.c MODULE_ALIAS("ishtp:{BD0FBCD9-F25E-B14D-4DE3-D7606A19CC4B}"); This issue has been unnoticed because the x86 kernel is most likely built natively on an x86 host. The guid field must not be reversed because guid_t is an array of __u8. Fixes: fa443bc3c1e4 ("HID: intel-ish-hid: add support for MODULE_DEVICE_TABLE()") Signed-off-by: Masahiro Yamada Reviewed-by: Thomas Weißschuh Tested-by: Srinivas Pandruvada Acked-by: Srinivas Pandruvada --- scripts/mod/file2alias.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'scripts/mod') diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c index 70bf6a2f585c..6583b36dbe69 100644 --- a/scripts/mod/file2alias.c +++ b/scripts/mod/file2alias.c @@ -1401,10 +1401,10 @@ static int do_mhi_ep_entry(const char *filename, void *symval, char *alias) /* Looks like: ishtp:{guid} */ static int do_ishtp_entry(const char *filename, void *symval, char *alias) { - DEF_FIELD(symval, ishtp_device_id, guid); + DEF_FIELD_ADDR(symval, ishtp_device_id, guid); strcpy(alias, ISHTP_MODULE_PREFIX "{"); - add_guid(alias, guid); + add_guid(alias, *guid); strcat(alias, "}"); return 1; -- cgit From bd78c9d714208fb5d989bd8ad007ff0e2bcfb2a9 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Sun, 8 Oct 2023 02:04:46 +0900 Subject: modpost: define TO_NATIVE() using bswap_* functions The current TO_NATIVE() has some limitations: 1) You cannot cast the argument. 2) You cannot pass a variable marked as 'const'. 3) Passing an array is a bug, but it is not detected. Impelement TO_NATIVE() using bswap_*() functions. These are GNU extensions. If we face portability issues, we can port the code from include/uapi/linux/swab.h. With this change, get_rel_type_and_sym() can be simplified by casting the arguments directly. Signed-off-by: Masahiro Yamada Reviewed-by: Nick Desaulniers --- scripts/mod/modpost.c | 13 ++++--------- scripts/mod/modpost.h | 25 ++++++++++++------------- 2 files changed, 16 insertions(+), 22 deletions(-) (limited to 'scripts/mod') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 2f3b0fe6f68d..99476a9695c5 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1410,15 +1410,10 @@ static void get_rel_type_and_sym(struct elf_info *elf, uint64_t r_info, return; } - if (is_64bit) { - Elf64_Xword r_info64 = r_info; - - r_info = TO_NATIVE(r_info64); - } else { - Elf32_Word r_info32 = r_info; - - r_info = TO_NATIVE(r_info32); - } + if (is_64bit) + r_info = TO_NATIVE((Elf64_Xword)r_info); + else + r_info = TO_NATIVE((Elf32_Word)r_info); *r_type = ELF_R_TYPE(r_info); *r_sym = ELF_R_SYM(r_info); diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h index 6413f26fcb6b..1392afec118c 100644 --- a/scripts/mod/modpost.h +++ b/scripts/mod/modpost.h @@ -1,4 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0 */ +#include #include #include #include @@ -51,21 +52,19 @@ #define ELF_R_TYPE ELF64_R_TYPE #endif -#if KERNEL_ELFDATA != HOST_ELFDATA +#define bswap(x) \ +({ \ + _Static_assert(sizeof(x) == 1 || sizeof(x) == 2 || \ + sizeof(x) == 4 || sizeof(x) == 8, "bug"); \ + (typeof(x))(sizeof(x) == 2 ? bswap_16(x) : \ + sizeof(x) == 4 ? bswap_32(x) : \ + sizeof(x) == 8 ? bswap_64(x) : \ + x); \ +}) -static inline void __endian(const void *src, void *dest, unsigned int size) -{ - unsigned int i; - for (i = 0; i < size; i++) - ((unsigned char*)dest)[i] = ((unsigned char*)src)[size - i-1]; -} +#if KERNEL_ELFDATA != HOST_ELFDATA -#define TO_NATIVE(x) \ -({ \ - typeof(x) __x; \ - __endian(&(x), &(__x), sizeof(__x)); \ - __x; \ -}) +#define TO_NATIVE(x) (bswap(x)) #else /* endianness matches */ -- cgit From 29ae5c02ed743c62e3c0619013b2bcd280bc562d Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Sun, 8 Oct 2023 02:04:47 +0900 Subject: modpost: refactor check_sec_ref() We can replace &elf->sechdrs[i] with &sechdrs[i] to slightly shorten the code because we already have the local variable 'sechdrs'. However, defining 'sechdr' instead shortens the code further. Signed-off-by: Masahiro Yamada Reviewed-by: Nick Desaulniers --- scripts/mod/modpost.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'scripts/mod') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 99476a9695c5..441d57ee3275 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1518,16 +1518,17 @@ static void section_rel(struct module *mod, struct elf_info *elf, static void check_sec_ref(struct module *mod, struct elf_info *elf) { int i; - Elf_Shdr *sechdrs = elf->sechdrs; /* Walk through all sections */ for (i = 0; i < elf->num_sections; i++) { - check_section(mod->name, elf, &elf->sechdrs[i]); + Elf_Shdr *sechdr = &elf->sechdrs[i]; + + check_section(mod->name, elf, sechdr); /* We want to process only relocation sections and not .init */ - if (sechdrs[i].sh_type == SHT_RELA) - section_rela(mod, elf, &elf->sechdrs[i]); - else if (sechdrs[i].sh_type == SHT_REL) - section_rel(mod, elf, &elf->sechdrs[i]); + if (sechdr->sh_type == SHT_RELA) + section_rela(mod, elf, sechdr); + else if (sechdr->sh_type == SHT_REL) + section_rel(mod, elf, sechdr); } } -- cgit From 77f9f57164561d0acbb9d6b2ca78280925f52aaa Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Sun, 8 Oct 2023 02:04:48 +0900 Subject: modpost: factor out the common boilerplate of section_rel(a) The first few lines of section_rel() and section_rela() are the same. They both retrieve the index of the section to which the relocaton applies, and skip known-good sections. This common code should be moved to check_sec_ref(). Avoid ugly casts when computing 'start' and 'stop', and also make the Elf_Rel and Elf_Rela pointers const. Signed-off-by: Masahiro Yamada Reviewed-by: Nick Desaulniers --- scripts/mod/modpost.c | 50 ++++++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 24 deletions(-) (limited to 'scripts/mod') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 441d57ee3275..f1f658122ad8 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1420,17 +1420,10 @@ static void get_rel_type_and_sym(struct elf_info *elf, uint64_t r_info, } static void section_rela(struct module *mod, struct elf_info *elf, - Elf_Shdr *sechdr) + unsigned int fsecndx, const char *fromsec, + const Elf_Rela *start, const Elf_Rela *stop) { - Elf_Rela *rela; - unsigned int fsecndx = sechdr->sh_info; - const char *fromsec = sec_name(elf, fsecndx); - Elf_Rela *start = (void *)elf->hdr + sechdr->sh_offset; - Elf_Rela *stop = (void *)start + sechdr->sh_size; - - /* if from section (name) is know good then skip it */ - if (match(fromsec, section_white_list)) - return; + const Elf_Rela *rela; for (rela = start; rela < stop; rela++) { Elf_Addr taddr, r_offset; @@ -1460,17 +1453,10 @@ static void section_rela(struct module *mod, struct elf_info *elf, } static void section_rel(struct module *mod, struct elf_info *elf, - Elf_Shdr *sechdr) + unsigned int fsecndx, const char *fromsec, + const Elf_Rel *start, const Elf_Rel *stop) { - Elf_Rel *rel; - unsigned int fsecndx = sechdr->sh_info; - const char *fromsec = sec_name(elf, fsecndx); - Elf_Rel *start = (void *)elf->hdr + sechdr->sh_offset; - Elf_Rel *stop = (void *)start + sechdr->sh_size; - - /* if from section (name) is know good then skip it */ - if (match(fromsec, section_white_list)) - return; + const Elf_Rel *rel; for (rel = start; rel < stop; rel++) { Elf_Sym *tsym; @@ -1525,10 +1511,26 @@ static void check_sec_ref(struct module *mod, struct elf_info *elf) check_section(mod->name, elf, sechdr); /* We want to process only relocation sections and not .init */ - if (sechdr->sh_type == SHT_RELA) - section_rela(mod, elf, sechdr); - else if (sechdr->sh_type == SHT_REL) - section_rel(mod, elf, sechdr); + if (sechdr->sh_type == SHT_REL || sechdr->sh_type == SHT_RELA) { + /* section to which the relocation applies */ + unsigned int secndx = sechdr->sh_info; + const char *secname = sec_name(elf, secndx); + const void *start, *stop; + + /* If the section is known good, skip it */ + if (match(secname, section_white_list)) + continue; + + start = sym_get_data_by_offset(elf, i, 0); + stop = start + sechdr->sh_size; + + if (sechdr->sh_type == SHT_RELA) + section_rela(mod, elf, secndx, secname, + start, stop); + else + section_rel(mod, elf, secndx, secname, + start, stop); + } } } -- cgit From 3ada34b0f6559b2388f1983366614fbe8027b6fd Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Mon, 23 Oct 2023 02:06:04 +0900 Subject: modpost: remove ALL_EXIT_DATA_SECTIONS macro This is unused. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'scripts/mod') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index f1f658122ad8..d936fa5fbbb1 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -794,8 +794,6 @@ static void check_section(const char *modname, struct elf_info *elf, #define ALL_INIT_DATA_SECTIONS \ ".init.setup", ".init.rodata", ".meminit.rodata", \ ".init.data", ".meminit.data" -#define ALL_EXIT_DATA_SECTIONS \ - ".exit.data", ".memexit.data" #define ALL_INIT_TEXT_SECTIONS \ ".init.text", ".meminit.text" -- cgit From 6a4e59eeedc3018cb57722eecfcbb49431aeb05f Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Mon, 23 Oct 2023 02:06:05 +0900 Subject: linux/init: remove __memexit* annotations We have never used __memexit, __memexitdata, or __memexitconst. These were unneeded. Signed-off-by: Masahiro Yamada Acked-by: Arnd Bergmann --- scripts/mod/modpost.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) (limited to 'scripts/mod') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index d936fa5fbbb1..bcc334b28a2c 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -798,7 +798,7 @@ static void check_section(const char *modname, struct elf_info *elf, #define ALL_INIT_TEXT_SECTIONS \ ".init.text", ".meminit.text" #define ALL_EXIT_TEXT_SECTIONS \ - ".exit.text", ".memexit.text" + ".exit.text" #define ALL_PCI_INIT_SECTIONS \ ".pci_fixup_early", ".pci_fixup_header", ".pci_fixup_final", \ @@ -806,10 +806,9 @@ static void check_section(const char *modname, struct elf_info *elf, ".pci_fixup_resume_early", ".pci_fixup_suspend" #define ALL_XXXINIT_SECTIONS MEM_INIT_SECTIONS -#define ALL_XXXEXIT_SECTIONS MEM_EXIT_SECTIONS #define ALL_INIT_SECTIONS INIT_SECTIONS, ALL_XXXINIT_SECTIONS -#define ALL_EXIT_SECTIONS EXIT_SECTIONS, ALL_XXXEXIT_SECTIONS +#define ALL_EXIT_SECTIONS EXIT_SECTIONS #define DATA_SECTIONS ".data", ".data.rel" #define TEXT_SECTIONS ".text", ".text.*", ".sched.text", \ @@ -822,7 +821,6 @@ static void check_section(const char *modname, struct elf_info *elf, #define MEM_INIT_SECTIONS ".meminit.*" #define EXIT_SECTIONS ".exit.*" -#define MEM_EXIT_SECTIONS ".memexit.*" #define ALL_TEXT_SECTIONS ALL_INIT_TEXT_SECTIONS, ALL_EXIT_TEXT_SECTIONS, \ TEXT_SECTIONS, OTHER_TEXT_SECTIONS @@ -832,7 +830,6 @@ enum mismatch { DATA_TO_ANY_INIT, TEXTDATA_TO_ANY_EXIT, XXXINIT_TO_SOME_INIT, - XXXEXIT_TO_SOME_EXIT, ANY_INIT_TO_ANY_EXIT, ANY_EXIT_TO_ANY_INIT, EXTABLE_TO_NON_TEXT, @@ -883,12 +880,6 @@ static const struct sectioncheck sectioncheck[] = { .bad_tosec = { INIT_SECTIONS, NULL }, .mismatch = XXXINIT_TO_SOME_INIT, }, -/* Do not reference exit code/data from memexit code/data */ -{ - .fromsec = { ALL_XXXEXIT_SECTIONS, NULL }, - .bad_tosec = { EXIT_SECTIONS, NULL }, - .mismatch = XXXEXIT_TO_SOME_EXIT, -}, /* Do not use exit code/data from init code */ { .fromsec = { ALL_INIT_SECTIONS, NULL }, @@ -1017,7 +1008,7 @@ static int secref_whitelist(const char *fromsec, const char *fromsym, /* symbols in data sections that may refer to meminit sections */ if (match(fromsec, PATTERNS(DATA_SECTIONS)) && - match(tosec, PATTERNS(ALL_XXXINIT_SECTIONS, ALL_XXXEXIT_SECTIONS)) && + match(tosec, PATTERNS(ALL_XXXINIT_SECTIONS)) && match(fromsym, PATTERNS("*driver"))) return 0; -- cgit From 50cccec15c48814765895891ca0d95d989b6a419 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Mon, 23 Oct 2023 02:06:06 +0900 Subject: modpost: disallow *driver to reference .meminit* sections Drivers must not reference .meminit* sections, which are discarded when CONFIG_MEMORY_HOTPLUG=n. The reason for whitelisting "*driver" in the section mismatch check was to allow drivers to reference symbols annotated as __devinit or __devexit that existed in the past. Those annotations were removed by the following commits: - 54b956b90360 ("Remove __dev* markings from init.h") - 92e9e6d1f984 ("modpost.c: Stop checking __dev* section mismatches") Remove the stale whitelist. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'scripts/mod') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index bcc334b28a2c..792ba9da0f27 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1006,12 +1006,6 @@ static int secref_whitelist(const char *fromsec, const char *fromsym, "*_console"))) return 0; - /* symbols in data sections that may refer to meminit sections */ - if (match(fromsec, PATTERNS(DATA_SECTIONS)) && - match(tosec, PATTERNS(ALL_XXXINIT_SECTIONS)) && - match(fromsym, PATTERNS("*driver"))) - return 0; - /* * symbols in data sections must not refer to .exit.*, but there are * quite a few offenders, so hide these unless for W=1 builds until -- cgit From e1dc1bfe5b27842c9e43a1f2d42c34decb0660c3 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Mon, 23 Oct 2023 02:06:07 +0900 Subject: modpost: remove more symbol patterns from the section check whitelist These symbol patterns were whitelisted to allow them to reference to functions with the old __devinit and __devexit annotations. We stopped doing this a long time ago, for example, commit 6f039790510f ("Drivers: scsi: remove __dev* attributes.") remove those annotations from the scsi drivers. Keep *_ops, *_probe, and *_console, otherwise they will really cause section mismatch warnings. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'scripts/mod') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 792ba9da0f27..4973b798a469 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -997,13 +997,7 @@ static int secref_whitelist(const char *fromsec, const char *fromsym, /* symbols in data sections that may refer to any init/exit sections */ if (match(fromsec, PATTERNS(DATA_SECTIONS)) && match(tosec, PATTERNS(ALL_INIT_SECTIONS, ALL_EXIT_SECTIONS)) && - match(fromsym, PATTERNS("*_template", // scsi uses *_template a lot - "*_timer", // arm uses ops structures named _timer a lot - "*_sht", // scsi also used *_sht to some extent - "*_ops", - "*_probe", - "*_probe_one", - "*_console"))) + match(fromsym, PATTERNS("*_ops", "*_probe", "*_console"))) return 0; /* -- cgit From 473a45bb35f080e31cb4fe45e905bfe3bd407fdf Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Mon, 23 Oct 2023 02:06:08 +0900 Subject: modpost: remove MEM_INIT_SECTIONS macro ALL_XXXINIT_SECTIONS and MEM_INIT_SECTIONS are the same. Remove the latter. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'scripts/mod') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 4973b798a469..54621a4bd421 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -805,7 +805,7 @@ static void check_section(const char *modname, struct elf_info *elf, ".pci_fixup_enable", ".pci_fixup_resume", \ ".pci_fixup_resume_early", ".pci_fixup_suspend" -#define ALL_XXXINIT_SECTIONS MEM_INIT_SECTIONS +#define ALL_XXXINIT_SECTIONS ".meminit.*" #define ALL_INIT_SECTIONS INIT_SECTIONS, ALL_XXXINIT_SECTIONS #define ALL_EXIT_SECTIONS EXIT_SECTIONS @@ -818,7 +818,6 @@ static void check_section(const char *modname, struct elf_info *elf, ".coldtext", ".softirqentry.text" #define INIT_SECTIONS ".init.*" -#define MEM_INIT_SECTIONS ".meminit.*" #define EXIT_SECTIONS ".exit.*" -- cgit From 48cd8df7afd1eef22cf7b125697a6d7c3d168c5c Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Mon, 23 Oct 2023 02:06:09 +0900 Subject: modpost: remove EXIT_SECTIONS macro ALL_EXIT_SECTIONS and EXIT_SECTIONS are the same. Remove the latter. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'scripts/mod') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 54621a4bd421..160336efedba 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -808,7 +808,7 @@ static void check_section(const char *modname, struct elf_info *elf, #define ALL_XXXINIT_SECTIONS ".meminit.*" #define ALL_INIT_SECTIONS INIT_SECTIONS, ALL_XXXINIT_SECTIONS -#define ALL_EXIT_SECTIONS EXIT_SECTIONS +#define ALL_EXIT_SECTIONS ".exit.*" #define DATA_SECTIONS ".data", ".data.rel" #define TEXT_SECTIONS ".text", ".text.*", ".sched.text", \ @@ -819,8 +819,6 @@ static void check_section(const char *modname, struct elf_info *elf, #define INIT_SECTIONS ".init.*" -#define EXIT_SECTIONS ".exit.*" - #define ALL_TEXT_SECTIONS ALL_INIT_TEXT_SECTIONS, ALL_EXIT_TEXT_SECTIONS, \ TEXT_SECTIONS, OTHER_TEXT_SECTIONS @@ -1006,7 +1004,7 @@ static int secref_whitelist(const char *fromsec, const char *fromsym, */ if (!extra_warn && match(fromsec, PATTERNS(DATA_SECTIONS)) && - match(tosec, PATTERNS(EXIT_SECTIONS)) && + match(tosec, PATTERNS(ALL_EXIT_SECTIONS)) && match(fromsym, PATTERNS("*driver"))) return 0; @@ -1169,7 +1167,7 @@ static void check_export_symbol(struct module *mod, struct elf_info *elf, if (match(secname, PATTERNS(INIT_SECTIONS))) warn("%s: %s: EXPORT_SYMBOL used for init symbol. Remove __init or EXPORT_SYMBOL.\n", mod->name, name); - else if (match(secname, PATTERNS(EXIT_SECTIONS))) + else if (match(secname, PATTERNS(ALL_EXIT_SECTIONS))) warn("%s: %s: EXPORT_SYMBOL used for exit symbol. Remove __exit or EXPORT_SYMBOL.\n", mod->name, name); } -- cgit From a3df1526da480c089c20868b7f4d486b9f266001 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Mon, 23 Oct 2023 02:06:10 +0900 Subject: modpost: disallow the combination of EXPORT_SYMBOL and __meminit* Theoretically, we could export conditionally-discarded code sections, such as .meminit*, if all the users can become modular under a certain condition. However, that would be difficult to control and such a tricky case has never occurred. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/mod') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 160336efedba..f76ae0909ccd 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1164,7 +1164,7 @@ static void check_export_symbol(struct module *mod, struct elf_info *elf, ELF_ST_TYPE(sym->st_info) == STT_LOPROC) s->is_func = true; - if (match(secname, PATTERNS(INIT_SECTIONS))) + if (match(secname, PATTERNS(ALL_INIT_SECTIONS))) warn("%s: %s: EXPORT_SYMBOL used for init symbol. Remove __init or EXPORT_SYMBOL.\n", mod->name, name); else if (match(secname, PATTERNS(ALL_EXIT_SECTIONS))) -- cgit From e578e4e3110635b20786e442baa3aeff9bb65f95 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Mon, 23 Oct 2023 02:06:11 +0900 Subject: modpost: use ALL_INIT_SECTIONS for the section check from DATA_SECTIONS ALL_INIT_SECTIONS is defined as follows: #define ALL_INIT_SECTIONS INIT_SECTIONS, ALL_XXXINIT_SECTIONS Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/mod') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index f76ae0909ccd..bc555ac5b47c 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -863,7 +863,7 @@ static const struct sectioncheck sectioncheck[] = { }, { .fromsec = { DATA_SECTIONS, NULL }, - .bad_tosec = { ALL_XXXINIT_SECTIONS, INIT_SECTIONS, NULL }, + .bad_tosec = { ALL_INIT_SECTIONS, NULL }, .mismatch = DATA_TO_ANY_INIT, }, { -- cgit From b3d4f446fc0f6bf6d50abfc403eb375d9b9f6378 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Mon, 23 Oct 2023 02:06:12 +0900 Subject: modpost: merge sectioncheck table entries regarding init/exit sections Check symbol references from normal sections to init/exit sections in a single entry. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) (limited to 'scripts/mod') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index bc555ac5b47c..144194c8f15b 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -823,9 +823,7 @@ static void check_section(const char *modname, struct elf_info *elf, TEXT_SECTIONS, OTHER_TEXT_SECTIONS enum mismatch { - TEXT_TO_ANY_INIT, - DATA_TO_ANY_INIT, - TEXTDATA_TO_ANY_EXIT, + TEXTDATA_TO_ANY_INIT_EXIT, XXXINIT_TO_SOME_INIT, ANY_INIT_TO_ANY_EXIT, ANY_EXIT_TO_ANY_INIT, @@ -856,20 +854,10 @@ static const struct sectioncheck sectioncheck[] = { /* Do not reference init/exit code/data from * normal code and data */ -{ - .fromsec = { TEXT_SECTIONS, NULL }, - .bad_tosec = { ALL_INIT_SECTIONS, NULL }, - .mismatch = TEXT_TO_ANY_INIT, -}, -{ - .fromsec = { DATA_SECTIONS, NULL }, - .bad_tosec = { ALL_INIT_SECTIONS, NULL }, - .mismatch = DATA_TO_ANY_INIT, -}, { .fromsec = { TEXT_SECTIONS, DATA_SECTIONS, NULL }, - .bad_tosec = { ALL_EXIT_SECTIONS, NULL }, - .mismatch = TEXTDATA_TO_ANY_EXIT, + .bad_tosec = { ALL_INIT_SECTIONS, ALL_EXIT_SECTIONS, NULL }, + .mismatch = TEXTDATA_TO_ANY_INIT_EXIT, }, /* Do not reference init code/data from meminit code/data */ { -- cgit From 34fcf231dcf94d7dea29c070228c4b93849f4850 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Mon, 23 Oct 2023 02:06:13 +0900 Subject: modpost: squash ALL_{INIT,EXIT}_TEXT_SECTIONS to ALL_TEXT_SECTIONS ALL_INIT_TEXT_SECTIONS and ALL_EXIT_TEXT_SECTIONS are only used in the macro definition of ALL_TEXT_SECTIONS. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'scripts/mod') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 144194c8f15b..973b5e5ae2dd 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -795,11 +795,6 @@ static void check_section(const char *modname, struct elf_info *elf, ".init.setup", ".init.rodata", ".meminit.rodata", \ ".init.data", ".meminit.data" -#define ALL_INIT_TEXT_SECTIONS \ - ".init.text", ".meminit.text" -#define ALL_EXIT_TEXT_SECTIONS \ - ".exit.text" - #define ALL_PCI_INIT_SECTIONS \ ".pci_fixup_early", ".pci_fixup_header", ".pci_fixup_final", \ ".pci_fixup_enable", ".pci_fixup_resume", \ @@ -819,7 +814,7 @@ static void check_section(const char *modname, struct elf_info *elf, #define INIT_SECTIONS ".init.*" -#define ALL_TEXT_SECTIONS ALL_INIT_TEXT_SECTIONS, ALL_EXIT_TEXT_SECTIONS, \ +#define ALL_TEXT_SECTIONS ".init.text", ".meminit.text", ".exit.text", \ TEXT_SECTIONS, OTHER_TEXT_SECTIONS enum mismatch { -- cgit