* [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h)
@ 2022-05-08 19:06 Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 01/14] modpost: remove left-over cross_compile declaration Masahiro Yamada
` (14 more replies)
0 siblings, 15 replies; 33+ messages in thread
From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev,
clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada
This is the third batch of cleanups in this development cycle.
Major changes in v4:
- Move static EXPORT_SYMBOL check to a script
- Some refactoring
Major changes in v3:
- Generate symbol CRCs as C code, and remove CONFIG_MODULE_REL_CRCS.
Major changes in v2:
- V1 did not work with CONFIG_MODULE_REL_CRCS.
I fixed this for v2.
- Reflect some review comments in v1
- Refactor the code more
- Avoid too long argument error
Masahiro Yamada (14):
modpost: remove left-over cross_compile declaration
modpost: change the license of EXPORT_SYMBOL to bool type
modpost: split the section mismatch checks into section-check.c
modpost: add sym_find_with_module() helper
modpost: extract symbol versions from *.cmd files
kbuild: link symbol CRCs at final link, removing
CONFIG_MODULE_REL_CRCS
kbuild: stop merging *.symversions
genksyms: adjust the output format to modpost
kbuild: do not create *.prelink.o for Clang LTO or IBT
kbuild: check static EXPORT_SYMBOL* by script instead of modpost
kbuild: make built-in.a rule robust against too long argument error
kbuild: make *.mod rule robust against too long argument error
kbuild: add cmd_and_savecmd macro
kbuild: rebuild multi-object modules when objtool is updated
arch/powerpc/Kconfig | 1 -
arch/s390/Kconfig | 1 -
arch/um/Kconfig | 1 -
include/asm-generic/export.h | 22 +-
include/linux/export-internal.h | 16 +
include/linux/export.h | 30 +-
init/Kconfig | 4 -
kernel/module.c | 10 +-
scripts/Kbuild.include | 10 +-
scripts/Makefile.build | 134 +--
scripts/Makefile.lib | 7 -
scripts/Makefile.modfinal | 5 +-
scripts/Makefile.modpost | 9 +-
scripts/check-local-export | 48 +
scripts/genksyms/genksyms.c | 18 +-
scripts/link-vmlinux.sh | 33 +-
scripts/mod/Makefile | 2 +-
scripts/mod/modpost.c | 1499 ++++---------------------------
scripts/mod/modpost.h | 35 +-
scripts/mod/section-check.c | 1222 +++++++++++++++++++++++++
20 files changed, 1551 insertions(+), 1556 deletions(-)
create mode 100644 include/linux/export-internal.h
create mode 100755 scripts/check-local-export
create mode 100644 scripts/mod/section-check.c
--
2.32.0
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH v4 01/14] modpost: remove left-over cross_compile declaration 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada @ 2022-05-08 19:06 ` Masahiro Yamada 2022-05-09 17:08 ` Nick Desaulniers 2022-05-12 4:35 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 02/14] modpost: change the license of EXPORT_SYMBOL to bool type Masahiro Yamada ` (13 subsequent siblings) 14 siblings, 2 replies; 33+ messages in thread From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada This is a remnant of commit 6543becf26ff ("mod/file2alias: make modalias generation safe for cross compiling"). Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- Changes in v4: - New patch scripts/mod/modpost.h | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h index cfa127d2bb8f..d9daeff07b83 100644 --- a/scripts/mod/modpost.h +++ b/scripts/mod/modpost.h @@ -174,7 +174,6 @@ static inline unsigned int get_secindex(const struct elf_info *info, } /* file2alias.c */ -extern unsigned int cross_build; void handle_moddevtable(struct module *mod, struct elf_info *info, Elf_Sym *sym, const char *symname); void add_moddevtable(struct buffer *buf, struct module *mod); -- 2.32.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v4 01/14] modpost: remove left-over cross_compile declaration 2022-05-08 19:06 ` [PATCH v4 01/14] modpost: remove left-over cross_compile declaration Masahiro Yamada @ 2022-05-09 17:08 ` Nick Desaulniers 2022-05-12 4:35 ` Masahiro Yamada 1 sibling, 0 replies; 33+ messages in thread From: Nick Desaulniers @ 2022-05-09 17:08 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, linux-kernel, Nathan Chancellor, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, Ard Biesheuvel, Sami Tolvanen, clang-built-linux On Sun, May 8, 2022 at 12:09 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > This is a remnant of commit 6543becf26ff ("mod/file2alias: make > modalias generation safe for cross compiling"). > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Thanks for the patch! (I wonder if we should put our old mailing list in .mailmap?) Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > --- > > Changes in v4: > - New patch > > scripts/mod/modpost.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h > index cfa127d2bb8f..d9daeff07b83 100644 > --- a/scripts/mod/modpost.h > +++ b/scripts/mod/modpost.h > @@ -174,7 +174,6 @@ static inline unsigned int get_secindex(const struct elf_info *info, > } > > /* file2alias.c */ > -extern unsigned int cross_build; > void handle_moddevtable(struct module *mod, struct elf_info *info, > Elf_Sym *sym, const char *symname); > void add_moddevtable(struct buffer *buf, struct module *mod); > -- -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 01/14] modpost: remove left-over cross_compile declaration 2022-05-08 19:06 ` [PATCH v4 01/14] modpost: remove left-over cross_compile declaration Masahiro Yamada 2022-05-09 17:08 ` Nick Desaulniers @ 2022-05-12 4:35 ` Masahiro Yamada 1 sibling, 0 replies; 33+ messages in thread From: Masahiro Yamada @ 2022-05-12 4:35 UTC (permalink / raw) To: Linux Kbuild mailing list Cc: Linux Kernel Mailing List, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen On Mon, May 9, 2022 at 4:09 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > This is a remnant of commit 6543becf26ff ("mod/file2alias: make > modalias generation safe for cross compiling"). > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- Applied to linux-kbuild. > > Changes in v4: > - New patch > > scripts/mod/modpost.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h > index cfa127d2bb8f..d9daeff07b83 100644 > --- a/scripts/mod/modpost.h > +++ b/scripts/mod/modpost.h > @@ -174,7 +174,6 @@ static inline unsigned int get_secindex(const struct elf_info *info, > } > > /* file2alias.c */ > -extern unsigned int cross_build; > void handle_moddevtable(struct module *mod, struct elf_info *info, > Elf_Sym *sym, const char *symname); > void add_moddevtable(struct buffer *buf, struct module *mod); > -- > 2.32.0 > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20220508190631.2386038-2-masahiroy%40kernel.org. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v4 02/14] modpost: change the license of EXPORT_SYMBOL to bool type 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 01/14] modpost: remove left-over cross_compile declaration Masahiro Yamada @ 2022-05-08 19:06 ` Masahiro Yamada 2022-05-12 4:36 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 03/14] modpost: split the section mismatch checks into section-check.c Masahiro Yamada ` (12 subsequent siblings) 14 siblings, 1 reply; 33+ messages in thread From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada There were more EXPORT_SYMBOL types in the past. The following commits removed unused ones. - f1c3d73e973c ("module: remove EXPORT_SYMBOL_GPL_FUTURE") - 367948220fce ("module: remove EXPORT_UNUSED_SYMBOL*") There are 3 remaining in enum export, but export_unknown does not make any sense because we never expect such a situation like "we do not know how it was exported". If the symbol name starts with "__ksymtab_", but the section name does not start with "___ksymtab+" or "___ksymtab_gpl+", it is not an exported symbol. It occurs when a variable starting with "__ksymtab_" is directly defined: int __ksymtab_foo; Presumably, there is no practical issue for using such a weird variable name (but there is no good reason for doing so, either). Anyway, that is not an exported symbol. Setting export_unknown is not the right thing to do. Do not call sym_add_exported() in this case. With pointless export_unknown removed, the export type finally becomes boolean (either EXPORT_SYMBOL or EXPORT_SYMBOL_GPL). I renamed the field name to is_gpl_only. EXPORT_SYMBOL_GPL sets it true. Only GPL-compatible modules can use it. I removed the orphan comment, "How a symbol is exported", which is unrelated to sec_mismatch_count. It is about enum export. See commit bd5cbcedf446 ("kbuild: export-type enhancement to modpost.c") Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> Tested-by: Nathan Chancellor <nathan@kernel.org> --- Changes in v4: - Rebase again because I dropped https://patchwork.kernel.org/project/linux-kbuild/patch/20220501084032.1025918-11-masahiroy@kernel.org/ - Remove warning message because I plan to change this hunk again in a later commit - Remove orphan comment Changes in v3: - New patch scripts/mod/modpost.c | 108 ++++++++++++------------------------------ 1 file changed, 30 insertions(+), 78 deletions(-) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index d9efbd5b31a6..a78b75f0eeb0 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -30,7 +30,7 @@ static bool all_versions; static bool external_module; /* Only warn about unresolved symbols */ static bool warn_unresolved; -/* How a symbol is exported */ + static int sec_mismatch_count; static bool sec_mismatch_warn_only = true; /* ignore missing files */ @@ -47,12 +47,6 @@ static bool error_occurred; #define MAX_UNRESOLVED_REPORTS 10 static unsigned int nr_unresolved; -enum export { - export_plain, - export_gpl, - export_unknown -}; - /* In kernel, this size is defined in linux/module.h; * here we use Elf_Addr instead of long for covering cross-compile */ @@ -219,7 +213,7 @@ struct symbol { bool crc_valid; bool weak; bool is_static; /* true if symbol is not global */ - enum export export; /* Type of export */ + bool is_gpl_only; /* exported by EXPORT_SYMBOL_GPL */ char name[]; }; @@ -316,34 +310,6 @@ static void add_namespace(struct list_head *head, const char *namespace) } } -static const struct { - const char *str; - enum export export; -} export_list[] = { - { .str = "EXPORT_SYMBOL", .export = export_plain }, - { .str = "EXPORT_SYMBOL_GPL", .export = export_gpl }, - { .str = "(unknown)", .export = export_unknown }, -}; - - -static const char *export_str(enum export ex) -{ - return export_list[ex].str; -} - -static enum export export_no(const char *s) -{ - int i; - - if (!s) - return export_unknown; - for (i = 0; export_list[i].export != export_unknown; i++) { - if (strcmp(export_list[i].str, s) == 0) - return export_list[i].export; - } - return export_unknown; -} - static void *sym_get_data_by_offset(const struct elf_info *info, unsigned int secindex, unsigned long offset) { @@ -374,18 +340,6 @@ static const char *sec_name(const struct elf_info *info, int secindex) #define strstarts(str, prefix) (strncmp(str, prefix, strlen(prefix)) == 0) -static enum export export_from_secname(struct elf_info *elf, unsigned int sec) -{ - const char *secname = sec_name(elf, sec); - - if (strstarts(secname, "___ksymtab+")) - return export_plain; - else if (strstarts(secname, "___ksymtab_gpl+")) - return export_gpl; - else - return export_unknown; -} - static void sym_update_namespace(const char *symname, const char *namespace) { struct symbol *s = find_symbol(symname); @@ -405,7 +359,7 @@ static void sym_update_namespace(const char *symname, const char *namespace) } static struct symbol *sym_add_exported(const char *name, struct module *mod, - enum export export) + bool gpl_only) { struct symbol *s = find_symbol(name); @@ -417,7 +371,7 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod, s = alloc_symbol(name); s->module = mod; - s->export = export; + s->is_gpl_only = gpl_only; list_add_tail(&s->list, &mod->exported_symbols); hash_add_symbol(s); @@ -689,8 +643,6 @@ static void handle_modversion(const struct module *mod, static void handle_symbol(struct module *mod, struct elf_info *info, const Elf_Sym *sym, const char *symname) { - const char *name; - switch (sym->st_shndx) { case SHN_COMMON: if (strstarts(symname, "__gnu_lto_")) { @@ -724,12 +676,15 @@ static void handle_symbol(struct module *mod, struct elf_info *info, default: /* All exported symbols */ if (strstarts(symname, "__ksymtab_")) { - enum export export; + const char *name, *secname; name = symname + strlen("__ksymtab_"); - export = export_from_secname(info, - get_secindex(info, sym)); - sym_add_exported(name, mod, export); + secname = sec_name(info, get_secindex(info, sym)); + + if (strstarts(secname, "___ksymtab_gpl+")) + sym_add_exported(name, mod, true); + else if (strstarts(secname, "___ksymtab+")) + sym_add_exported(name, mod, false); } if (strcmp(symname, "init_module") == 0) mod->has_init = true; @@ -2140,20 +2095,6 @@ void buf_write(struct buffer *buf, const char *s, int len) buf->pos += len; } -static void check_for_gpl_usage(enum export exp, const char *m, const char *s) -{ - switch (exp) { - case export_gpl: - error("GPL-incompatible module %s.ko uses GPL-only symbol '%s'\n", - m, s); - break; - case export_plain: - case export_unknown: - /* ignore */ - break; - } -} - static void check_exports(struct module *mod) { struct symbol *s, *exp; @@ -2192,8 +2133,9 @@ static void check_exports(struct module *mod) add_namespace(&mod->missing_namespaces, exp->namespace); } - if (!mod->is_gpl_compatible) - check_for_gpl_usage(exp->export, basename, exp->name); + if (!mod->is_gpl_compatible && exp->is_gpl_only) + error("GPL-incompatible module %s.ko uses GPL-only symbol '%s'\n", + basename, exp->name); } } @@ -2437,6 +2379,7 @@ static void read_dump(const char *fname) unsigned int crc; struct module *mod; struct symbol *s; + bool gpl_only; if (!(symname = strchr(line, '\t'))) goto fail; @@ -2454,12 +2397,22 @@ static void read_dump(const char *fname) crc = strtoul(line, &d, 16); if (*symname == '\0' || *modname == '\0' || *d != '\0') goto fail; + + if (!strcmp(export, "EXPORT_SYMBOL_GPL")) { + gpl_only = true; + } else if (!strcmp(export, "EXPORT_SYMBOL")) { + gpl_only = false; + } else { + error("%s: unknown license %s. skip", symname, export); + continue; + } + mod = find_module(modname); if (!mod) { mod = new_module(modname); mod->from_dump = true; } - s = sym_add_exported(symname, mod, export_no(export)); + s = sym_add_exported(symname, mod, gpl_only); s->is_static = false; sym_set_crc(symname, crc); sym_update_namespace(symname, namespace); @@ -2481,9 +2434,9 @@ static void write_dump(const char *fname) if (mod->from_dump) continue; list_for_each_entry(sym, &mod->exported_symbols, list) { - buf_printf(&buf, "0x%08x\t%s\t%s\t%s\t%s\n", + buf_printf(&buf, "0x%08x\t%s\t%s\tEXPORT_SYMBOL%s\t%s\n", sym->crc, sym->name, mod->name, - export_str(sym->export), + sym->is_gpl_only ? "_GPL" : "", sym->namespace ?: ""); } } @@ -2604,9 +2557,8 @@ int main(int argc, char **argv) for (s = symbolhash[n]; s; s = s->next) { if (s->is_static) - error("\"%s\" [%s] is a static %s\n", - s->name, s->module->name, - export_str(s->export)); + error("\"%s\" [%s] is a static EXPORT_SYMBOL\n", + s->name, s->module->name); } } -- 2.32.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v4 02/14] modpost: change the license of EXPORT_SYMBOL to bool type 2022-05-08 19:06 ` [PATCH v4 02/14] modpost: change the license of EXPORT_SYMBOL to bool type Masahiro Yamada @ 2022-05-12 4:36 ` Masahiro Yamada 0 siblings, 0 replies; 33+ messages in thread From: Masahiro Yamada @ 2022-05-12 4:36 UTC (permalink / raw) To: Linux Kbuild mailing list Cc: Linux Kernel Mailing List, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen On Mon, May 9, 2022 at 4:09 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > There were more EXPORT_SYMBOL types in the past. The following commits > removed unused ones. > > - f1c3d73e973c ("module: remove EXPORT_SYMBOL_GPL_FUTURE") > - 367948220fce ("module: remove EXPORT_UNUSED_SYMBOL*") > > There are 3 remaining in enum export, but export_unknown does not make > any sense because we never expect such a situation like "we do not know > how it was exported". > > If the symbol name starts with "__ksymtab_", but the section name > does not start with "___ksymtab+" or "___ksymtab_gpl+", it is not an > exported symbol. > > It occurs when a variable starting with "__ksymtab_" is directly defined: > > int __ksymtab_foo; > > Presumably, there is no practical issue for using such a weird variable > name (but there is no good reason for doing so, either). > > Anyway, that is not an exported symbol. Setting export_unknown is not > the right thing to do. Do not call sym_add_exported() in this case. > > With pointless export_unknown removed, the export type finally becomes > boolean (either EXPORT_SYMBOL or EXPORT_SYMBOL_GPL). > > I renamed the field name to is_gpl_only. EXPORT_SYMBOL_GPL sets it true. > Only GPL-compatible modules can use it. > > I removed the orphan comment, "How a symbol is exported", which is > unrelated to sec_mismatch_count. It is about enum export. > See commit bd5cbcedf446 ("kbuild: export-type enhancement to modpost.c") > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> > Tested-by: Nathan Chancellor <nathan@kernel.org> > --- Applied to linux-kbuild. > > Changes in v4: > - Rebase again because I dropped > https://patchwork.kernel.org/project/linux-kbuild/patch/20220501084032.1025918-11-masahiroy@kernel.org/ > - Remove warning message because I plan to change this hunk again in a later commit > - Remove orphan comment > > Changes in v3: > - New patch > > scripts/mod/modpost.c | 108 ++++++++++++------------------------------ > 1 file changed, 30 insertions(+), 78 deletions(-) > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index d9efbd5b31a6..a78b75f0eeb0 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -30,7 +30,7 @@ static bool all_versions; > static bool external_module; > /* Only warn about unresolved symbols */ > static bool warn_unresolved; > -/* How a symbol is exported */ > + > static int sec_mismatch_count; > static bool sec_mismatch_warn_only = true; > /* ignore missing files */ > @@ -47,12 +47,6 @@ static bool error_occurred; > #define MAX_UNRESOLVED_REPORTS 10 > static unsigned int nr_unresolved; > > -enum export { > - export_plain, > - export_gpl, > - export_unknown > -}; > - > /* In kernel, this size is defined in linux/module.h; > * here we use Elf_Addr instead of long for covering cross-compile > */ > @@ -219,7 +213,7 @@ struct symbol { > bool crc_valid; > bool weak; > bool is_static; /* true if symbol is not global */ > - enum export export; /* Type of export */ > + bool is_gpl_only; /* exported by EXPORT_SYMBOL_GPL */ > char name[]; > }; > > @@ -316,34 +310,6 @@ static void add_namespace(struct list_head *head, const char *namespace) > } > } > > -static const struct { > - const char *str; > - enum export export; > -} export_list[] = { > - { .str = "EXPORT_SYMBOL", .export = export_plain }, > - { .str = "EXPORT_SYMBOL_GPL", .export = export_gpl }, > - { .str = "(unknown)", .export = export_unknown }, > -}; > - > - > -static const char *export_str(enum export ex) > -{ > - return export_list[ex].str; > -} > - > -static enum export export_no(const char *s) > -{ > - int i; > - > - if (!s) > - return export_unknown; > - for (i = 0; export_list[i].export != export_unknown; i++) { > - if (strcmp(export_list[i].str, s) == 0) > - return export_list[i].export; > - } > - return export_unknown; > -} > - > static void *sym_get_data_by_offset(const struct elf_info *info, > unsigned int secindex, unsigned long offset) > { > @@ -374,18 +340,6 @@ static const char *sec_name(const struct elf_info *info, int secindex) > > #define strstarts(str, prefix) (strncmp(str, prefix, strlen(prefix)) == 0) > > -static enum export export_from_secname(struct elf_info *elf, unsigned int sec) > -{ > - const char *secname = sec_name(elf, sec); > - > - if (strstarts(secname, "___ksymtab+")) > - return export_plain; > - else if (strstarts(secname, "___ksymtab_gpl+")) > - return export_gpl; > - else > - return export_unknown; > -} > - > static void sym_update_namespace(const char *symname, const char *namespace) > { > struct symbol *s = find_symbol(symname); > @@ -405,7 +359,7 @@ static void sym_update_namespace(const char *symname, const char *namespace) > } > > static struct symbol *sym_add_exported(const char *name, struct module *mod, > - enum export export) > + bool gpl_only) > { > struct symbol *s = find_symbol(name); > > @@ -417,7 +371,7 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod, > > s = alloc_symbol(name); > s->module = mod; > - s->export = export; > + s->is_gpl_only = gpl_only; > list_add_tail(&s->list, &mod->exported_symbols); > hash_add_symbol(s); > > @@ -689,8 +643,6 @@ static void handle_modversion(const struct module *mod, > static void handle_symbol(struct module *mod, struct elf_info *info, > const Elf_Sym *sym, const char *symname) > { > - const char *name; > - > switch (sym->st_shndx) { > case SHN_COMMON: > if (strstarts(symname, "__gnu_lto_")) { > @@ -724,12 +676,15 @@ static void handle_symbol(struct module *mod, struct elf_info *info, > default: > /* All exported symbols */ > if (strstarts(symname, "__ksymtab_")) { > - enum export export; > + const char *name, *secname; > > name = symname + strlen("__ksymtab_"); > - export = export_from_secname(info, > - get_secindex(info, sym)); > - sym_add_exported(name, mod, export); > + secname = sec_name(info, get_secindex(info, sym)); > + > + if (strstarts(secname, "___ksymtab_gpl+")) > + sym_add_exported(name, mod, true); > + else if (strstarts(secname, "___ksymtab+")) > + sym_add_exported(name, mod, false); > } > if (strcmp(symname, "init_module") == 0) > mod->has_init = true; > @@ -2140,20 +2095,6 @@ void buf_write(struct buffer *buf, const char *s, int len) > buf->pos += len; > } > > -static void check_for_gpl_usage(enum export exp, const char *m, const char *s) > -{ > - switch (exp) { > - case export_gpl: > - error("GPL-incompatible module %s.ko uses GPL-only symbol '%s'\n", > - m, s); > - break; > - case export_plain: > - case export_unknown: > - /* ignore */ > - break; > - } > -} > - > static void check_exports(struct module *mod) > { > struct symbol *s, *exp; > @@ -2192,8 +2133,9 @@ static void check_exports(struct module *mod) > add_namespace(&mod->missing_namespaces, exp->namespace); > } > > - if (!mod->is_gpl_compatible) > - check_for_gpl_usage(exp->export, basename, exp->name); > + if (!mod->is_gpl_compatible && exp->is_gpl_only) > + error("GPL-incompatible module %s.ko uses GPL-only symbol '%s'\n", > + basename, exp->name); > } > } > > @@ -2437,6 +2379,7 @@ static void read_dump(const char *fname) > unsigned int crc; > struct module *mod; > struct symbol *s; > + bool gpl_only; > > if (!(symname = strchr(line, '\t'))) > goto fail; > @@ -2454,12 +2397,22 @@ static void read_dump(const char *fname) > crc = strtoul(line, &d, 16); > if (*symname == '\0' || *modname == '\0' || *d != '\0') > goto fail; > + > + if (!strcmp(export, "EXPORT_SYMBOL_GPL")) { > + gpl_only = true; > + } else if (!strcmp(export, "EXPORT_SYMBOL")) { > + gpl_only = false; > + } else { > + error("%s: unknown license %s. skip", symname, export); > + continue; > + } > + > mod = find_module(modname); > if (!mod) { > mod = new_module(modname); > mod->from_dump = true; > } > - s = sym_add_exported(symname, mod, export_no(export)); > + s = sym_add_exported(symname, mod, gpl_only); > s->is_static = false; > sym_set_crc(symname, crc); > sym_update_namespace(symname, namespace); > @@ -2481,9 +2434,9 @@ static void write_dump(const char *fname) > if (mod->from_dump) > continue; > list_for_each_entry(sym, &mod->exported_symbols, list) { > - buf_printf(&buf, "0x%08x\t%s\t%s\t%s\t%s\n", > + buf_printf(&buf, "0x%08x\t%s\t%s\tEXPORT_SYMBOL%s\t%s\n", > sym->crc, sym->name, mod->name, > - export_str(sym->export), > + sym->is_gpl_only ? "_GPL" : "", > sym->namespace ?: ""); > } > } > @@ -2604,9 +2557,8 @@ int main(int argc, char **argv) > > for (s = symbolhash[n]; s; s = s->next) { > if (s->is_static) > - error("\"%s\" [%s] is a static %s\n", > - s->name, s->module->name, > - export_str(s->export)); > + error("\"%s\" [%s] is a static EXPORT_SYMBOL\n", > + s->name, s->module->name); > } > } > > -- > 2.32.0 > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20220508190631.2386038-3-masahiroy%40kernel.org. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v4 03/14] modpost: split the section mismatch checks into section-check.c 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 01/14] modpost: remove left-over cross_compile declaration Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 02/14] modpost: change the license of EXPORT_SYMBOL to bool type Masahiro Yamada @ 2022-05-08 19:06 ` Masahiro Yamada 2022-05-09 17:19 ` Nick Desaulniers 2022-05-08 19:06 ` [PATCH v4 04/14] modpost: add sym_find_with_module() helper Masahiro Yamada ` (11 subsequent siblings) 14 siblings, 1 reply; 33+ messages in thread From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada modpost.c is too big, and the half of the code is for section checks. Split it. I fixed some style issues in the moved code. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- Changes in v4: - New patch scripts/mod/Makefile | 2 +- scripts/mod/modpost.c | 1202 +--------------------------------- scripts/mod/modpost.h | 34 +- scripts/mod/section-check.c | 1222 +++++++++++++++++++++++++++++++++++ 4 files changed, 1240 insertions(+), 1220 deletions(-) create mode 100644 scripts/mod/section-check.c diff --git a/scripts/mod/Makefile b/scripts/mod/Makefile index c9e38ad937fd..ca739c6c68a1 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 section-check.o file2alias.o sumversion.o devicetable-offsets-file := devicetable-offsets.h diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index a78b75f0eeb0..e7e2c70a98f5 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -31,7 +31,7 @@ static bool external_module; /* Only warn about unresolved symbols */ static bool warn_unresolved; -static int sec_mismatch_count; +int sec_mismatch_count; static bool sec_mismatch_warn_only = true; /* ignore missing files */ static bool ignore_missing_files; @@ -310,8 +310,8 @@ static void add_namespace(struct list_head *head, const char *namespace) } } -static void *sym_get_data_by_offset(const struct elf_info *info, - unsigned int secindex, unsigned long offset) +void *sym_get_data_by_offset(const struct elf_info *info, + unsigned int secindex, unsigned long offset) { Elf_Shdr *sechdr = &info->sechdrs[secindex]; @@ -327,19 +327,17 @@ static void *sym_get_data(const struct elf_info *info, const Elf_Sym *sym) sym->st_value); } -static const char *sech_name(const struct elf_info *info, Elf_Shdr *sechdr) +const char *sech_name(const struct elf_info *info, Elf_Shdr *sechdr) { return sym_get_data_by_offset(info, info->secindex_strings, sechdr->sh_name); } -static const char *sec_name(const struct elf_info *info, int secindex) +const char *sec_name(const struct elf_info *info, int secindex) { return sech_name(info, &info->sechdrs[secindex]); } -#define strstarts(str, prefix) (strncmp(str, prefix, strlen(prefix)) == 0) - static void sym_update_namespace(const char *symname, const char *namespace) { struct symbol *s = find_symbol(symname); @@ -741,1196 +739,6 @@ static char *get_modinfo(struct elf_info *info, const char *tag) return get_next_modinfo(info, tag, NULL); } -/** - * Test if string s ends in string sub - * return 0 if match - **/ -static int strrcmp(const char *s, const char *sub) -{ - int slen, sublen; - - if (!s || !sub) - return 1; - - slen = strlen(s); - sublen = strlen(sub); - - if ((slen == 0) || (sublen == 0)) - return 1; - - if (sublen > slen) - return 1; - - return memcmp(s + slen - sublen, sub, sublen); -} - -static const char *sym_name(struct elf_info *elf, Elf_Sym *sym) -{ - if (sym) - return elf->strtab + sym->st_name; - else - return "(unknown)"; -} - -/* The pattern is an array of simple patterns. - * "foo" will match an exact string equal to "foo" - * "*foo" will match a string that ends with "foo" - * "foo*" will match a string that begins with "foo" - * "*foo*" will match a string that contains "foo" - */ -static int match(const char *sym, const char * const pat[]) -{ - const char *p; - while (*pat) { - const char *endp; - - p = *pat++; - endp = p + strlen(p) - 1; - - /* "*foo*" */ - if (*p == '*' && *endp == '*') { - char *bare = NOFAIL(strndup(p + 1, strlen(p) - 2)); - char *here = strstr(sym, bare); - - free(bare); - if (here != NULL) - return 1; - } - /* "*foo" */ - else if (*p == '*') { - if (strrcmp(sym, p + 1) == 0) - return 1; - } - /* "foo*" */ - else if (*endp == '*') { - if (strncmp(sym, p, strlen(p) - 1) == 0) - return 1; - } - /* no wildcards */ - else { - if (strcmp(p, sym) == 0) - return 1; - } - } - /* no match */ - return 0; -} - -/* sections that we do not want to do full section mismatch check on */ -static const char *const section_white_list[] = -{ - ".comment*", - ".debug*", - ".cranges", /* sh64 */ - ".zdebug*", /* Compressed debug sections. */ - ".GCC.command.line", /* record-gcc-switches */ - ".mdebug*", /* alpha, score, mips etc. */ - ".pdr", /* alpha, score, mips etc. */ - ".stab*", - ".note*", - ".got*", - ".toc*", - ".xt.prop", /* xtensa */ - ".xt.lit", /* xtensa */ - ".arcextmap*", /* arc */ - ".gnu.linkonce.arcext*", /* arc : modules */ - ".cmem*", /* EZchip */ - ".fmt_slot*", /* EZchip */ - ".gnu.lto*", - ".discard.*", - NULL -}; - -/* - * This is used to find sections missing the SHF_ALLOC flag. - * The cause of this is often a section specified in assembler - * without "ax" / "aw". - */ -static void check_section(const char *modname, struct elf_info *elf, - Elf_Shdr *sechdr) -{ - const char *sec = sech_name(elf, sechdr); - - if (sechdr->sh_type == SHT_PROGBITS && - !(sechdr->sh_flags & SHF_ALLOC) && - !match(sec, section_white_list)) { - warn("%s (%s): unexpected non-allocatable section.\n" - "Did you forget to use \"ax\"/\"aw\" in a .S file?\n" - "Note that for example <linux/init.h> contains\n" - "section definitions for use in .S files.\n\n", - modname, sec); - } -} - - - -#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" -#define ALL_EXIT_TEXT_SECTIONS \ - ".exit.text", ".memexit.text" - -#define ALL_PCI_INIT_SECTIONS \ - ".pci_fixup_early", ".pci_fixup_header", ".pci_fixup_final", \ - ".pci_fixup_enable", ".pci_fixup_resume", \ - ".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 DATA_SECTIONS ".data", ".data.rel" -#define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \ - ".kprobes.text", ".cpuidle.text", ".noinstr.text" -#define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \ - ".fixup", ".entry.text", ".exception.text", ".text.*", \ - ".coldtext", ".softirqentry.text" - -#define INIT_SECTIONS ".init.*" -#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 - -/* init data sections */ -static const char *const init_data_sections[] = - { ALL_INIT_DATA_SECTIONS, NULL }; - -/* all init sections */ -static const char *const init_sections[] = { ALL_INIT_SECTIONS, NULL }; - -/* All init and exit sections (code + data) */ -static const char *const init_exit_sections[] = - {ALL_INIT_SECTIONS, ALL_EXIT_SECTIONS, NULL }; - -/* all text sections */ -static const char *const text_sections[] = { ALL_TEXT_SECTIONS, NULL }; - -/* data section */ -static const char *const data_sections[] = { DATA_SECTIONS, NULL }; - - -/* symbols in .data that may refer to init/exit sections */ -#define DEFAULT_SYMBOL_WHITE_LIST \ - "*driver", \ - "*_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" - -static const char *const head_sections[] = { ".head.text*", NULL }; -static const char *const linker_symbols[] = - { "__init_begin", "_sinittext", "_einittext", NULL }; -static const char *const optim_symbols[] = { "*.constprop.*", NULL }; - -enum mismatch { - TEXT_TO_ANY_INIT, - DATA_TO_ANY_INIT, - TEXT_TO_ANY_EXIT, - DATA_TO_ANY_EXIT, - XXXINIT_TO_SOME_INIT, - XXXEXIT_TO_SOME_EXIT, - ANY_INIT_TO_ANY_EXIT, - ANY_EXIT_TO_ANY_INIT, - EXPORT_TO_INIT_EXIT, - EXTABLE_TO_NON_TEXT, -}; - -/** - * Describe how to match sections on different criteria: - * - * @fromsec: Array of sections to be matched. - * - * @bad_tosec: Relocations applied to a section in @fromsec to a section in - * this array is forbidden (black-list). Can be empty. - * - * @good_tosec: Relocations applied to a section in @fromsec must be - * targeting sections in this array (white-list). Can be empty. - * - * @mismatch: Type of mismatch. - * - * @symbol_white_list: Do not match a relocation to a symbol in this list - * even if it is targeting a section in @bad_to_sec. - * - * @handler: Specific handler to call when a match is found. If NULL, - * default_mismatch_handler() will be called. - * - */ -struct sectioncheck { - const char *fromsec[20]; - const char *bad_tosec[20]; - const char *good_tosec[20]; - enum mismatch mismatch; - const char *symbol_white_list[20]; - void (*handler)(const char *modname, struct elf_info *elf, - const struct sectioncheck* const mismatch, - Elf_Rela *r, Elf_Sym *sym, const char *fromsec); - -}; - -static void extable_mismatch_handler(const char *modname, struct elf_info *elf, - const struct sectioncheck* const mismatch, - Elf_Rela *r, Elf_Sym *sym, - const char *fromsec); - -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, - .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, -}, -{ - .fromsec = { DATA_SECTIONS, NULL }, - .bad_tosec = { ALL_XXXINIT_SECTIONS, NULL }, - .mismatch = DATA_TO_ANY_INIT, - .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, -}, -{ - .fromsec = { DATA_SECTIONS, NULL }, - .bad_tosec = { INIT_SECTIONS, NULL }, - .mismatch = DATA_TO_ANY_INIT, - .symbol_white_list = { - "*_template", "*_timer", "*_sht", "*_ops", - "*_probe", "*_probe_one", "*_console", NULL - }, -}, -{ - .fromsec = { TEXT_SECTIONS, NULL }, - .bad_tosec = { ALL_EXIT_SECTIONS, NULL }, - .mismatch = TEXT_TO_ANY_EXIT, - .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, -}, -{ - .fromsec = { DATA_SECTIONS, NULL }, - .bad_tosec = { ALL_EXIT_SECTIONS, NULL }, - .mismatch = DATA_TO_ANY_EXIT, - .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, -}, -/* Do not reference init code/data from meminit code/data */ -{ - .fromsec = { ALL_XXXINIT_SECTIONS, NULL }, - .bad_tosec = { INIT_SECTIONS, NULL }, - .mismatch = XXXINIT_TO_SOME_INIT, - .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, -}, -/* 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, - .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, -}, -/* Do not use exit code/data from init code */ -{ - .fromsec = { ALL_INIT_SECTIONS, NULL }, - .bad_tosec = { ALL_EXIT_SECTIONS, NULL }, - .mismatch = ANY_INIT_TO_ANY_EXIT, - .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, -}, -/* Do not use init code/data from exit code */ -{ - .fromsec = { ALL_EXIT_SECTIONS, NULL }, - .bad_tosec = { ALL_INIT_SECTIONS, NULL }, - .mismatch = ANY_EXIT_TO_ANY_INIT, - .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, -}, -{ - .fromsec = { ALL_PCI_INIT_SECTIONS, NULL }, - .bad_tosec = { INIT_SECTIONS, NULL }, - .mismatch = ANY_INIT_TO_ANY_EXIT, - .symbol_white_list = { NULL }, -}, -/* Do not export init/exit functions or data */ -{ - .fromsec = { "__ksymtab*", NULL }, - .bad_tosec = { INIT_SECTIONS, EXIT_SECTIONS, NULL }, - .mismatch = EXPORT_TO_INIT_EXIT, - .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, -}, -{ - .fromsec = { "__ex_table", NULL }, - /* If you're adding any new black-listed sections in here, consider - * adding a special 'printer' for them in scripts/check_extable. - */ - .bad_tosec = { ".altinstr_replacement", NULL }, - .good_tosec = {ALL_TEXT_SECTIONS , NULL}, - .mismatch = EXTABLE_TO_NON_TEXT, - .handler = extable_mismatch_handler, -} -}; - -static const struct sectioncheck *section_mismatch( - const char *fromsec, const char *tosec) -{ - int i; - int elems = sizeof(sectioncheck) / sizeof(struct sectioncheck); - const struct sectioncheck *check = §ioncheck[0]; - - /* - * The target section could be the SHT_NUL section when we're - * handling relocations to un-resolved symbols, trying to match it - * doesn't make much sense and causes build failures on parisc - * architectures. - */ - if (*tosec == '\0') - return NULL; - - for (i = 0; i < elems; i++) { - if (match(fromsec, check->fromsec)) { - if (check->bad_tosec[0] && match(tosec, check->bad_tosec)) - return check; - if (check->good_tosec[0] && !match(tosec, check->good_tosec)) - return check; - } - check++; - } - return NULL; -} - -/** - * Whitelist to allow certain references to pass with no warning. - * - * Pattern 1: - * If a module parameter is declared __initdata and permissions=0 - * then this is legal despite the warning generated. - * We cannot see value of permissions here, so just ignore - * this pattern. - * The pattern is identified by: - * tosec = .init.data - * fromsec = .data* - * atsym =__param* - * - * Pattern 1a: - * module_param_call() ops can refer to __init set function if permissions=0 - * The pattern is identified by: - * tosec = .init.text - * fromsec = .data* - * atsym = __param_ops_* - * - * Pattern 2: - * Many drivers utilise a *driver container with references to - * add, remove, probe functions etc. - * the pattern is identified by: - * tosec = init or exit section - * fromsec = data section - * atsym = *driver, *_template, *_sht, *_ops, *_probe, - * *probe_one, *_console, *_timer - * - * Pattern 3: - * Whitelist all references from .head.text to any init section - * - * Pattern 4: - * Some symbols belong to init section but still it is ok to reference - * these from non-init sections as these symbols don't have any memory - * allocated for them and symbol address and value are same. So even - * if init section is freed, its ok to reference those symbols. - * For ex. symbols marking the init section boundaries. - * This pattern is identified by - * refsymname = __init_begin, _sinittext, _einittext - * - * Pattern 5: - * GCC may optimize static inlines when fed constant arg(s) resulting - * in functions like cpumask_empty() -- generating an associated symbol - * cpumask_empty.constprop.3 that appears in the audit. If the const that - * is passed in comes from __init, like say nmi_ipi_mask, we get a - * meaningless section warning. May need to add isra symbols too... - * This pattern is identified by - * tosec = init section - * fromsec = text section - * refsymname = *.constprop.* - * - * Pattern 6: - * Hide section mismatch warnings for ELF local symbols. The goal - * is to eliminate false positive modpost warnings caused by - * compiler-generated ELF local symbol names such as ".LANCHOR1". - * Autogenerated symbol names bypass modpost's "Pattern 2" - * whitelisting, which relies on pattern-matching against symbol - * names to work. (One situation where gcc can autogenerate ELF - * local symbols is when "-fsection-anchors" is used.) - **/ -static int secref_whitelist(const struct sectioncheck *mismatch, - const char *fromsec, const char *fromsym, - const char *tosec, const char *tosym) -{ - /* Check for pattern 1 */ - if (match(tosec, init_data_sections) && - match(fromsec, data_sections) && - strstarts(fromsym, "__param")) - return 0; - - /* Check for pattern 1a */ - if (strcmp(tosec, ".init.text") == 0 && - match(fromsec, data_sections) && - strstarts(fromsym, "__param_ops_")) - return 0; - - /* Check for pattern 2 */ - if (match(tosec, init_exit_sections) && - match(fromsec, data_sections) && - match(fromsym, mismatch->symbol_white_list)) - return 0; - - /* Check for pattern 3 */ - if (match(fromsec, head_sections) && - match(tosec, init_sections)) - return 0; - - /* Check for pattern 4 */ - if (match(tosym, linker_symbols)) - return 0; - - /* Check for pattern 5 */ - if (match(fromsec, text_sections) && - match(tosec, init_sections) && - match(fromsym, optim_symbols)) - return 0; - - /* Check for pattern 6 */ - if (strstarts(fromsym, ".L")) - return 0; - - return 1; -} - -static inline int is_arm_mapping_symbol(const char *str) -{ - return str[0] == '$' && strchr("axtd", str[1]) - && (str[2] == '\0' || str[2] == '.'); -} - -/* - * If there's no name there, ignore it; likewise, ignore it if it's - * one of the magic symbols emitted used by current ARM 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_arm_mapping_symbol(name); -} - -/** - * Find symbol based on relocation record info. - * In some cases the symbol supplied is a valid symbol so - * return refsym. If st_name != 0 we assume this is a valid symbol. - * In other cases the symbol needs to be looked up in the symbol table - * based on section and address. - * **/ -static Elf_Sym *find_elf_symbol(struct elf_info *elf, Elf64_Sword addr, - Elf_Sym *relsym) -{ - Elf_Sym *sym; - Elf_Sym *near = NULL; - Elf64_Sword distance = 20; - Elf64_Sword d; - unsigned int relsym_secindex; - - if (relsym->st_name != 0) - return relsym; - - relsym_secindex = get_secindex(elf, relsym); - for (sym = elf->symtab_start; sym < elf->symtab_stop; sym++) { - if (get_secindex(elf, sym) != relsym_secindex) - continue; - if (ELF_ST_TYPE(sym->st_info) == STT_SECTION) - continue; - if (!is_valid_name(elf, sym)) - continue; - if (sym->st_value == addr) - return sym; - /* Find a symbol nearby - addr are maybe negative */ - d = sym->st_value - addr; - if (d < 0) - d = addr - sym->st_value; - if (d < distance) { - distance = d; - near = sym; - } - } - /* We need a close match */ - if (distance < 20) - return near; - else - return NULL; -} - -/* - * Find symbols before or equal addr and after addr - in the section sec. - * If we find two symbols with equal offset prefer one with a valid name. - * The ELF format may have a better way to detect what type of symbol - * it is, but this works for now. - **/ -static Elf_Sym *find_elf_symbol2(struct elf_info *elf, Elf_Addr addr, - const char *sec) -{ - Elf_Sym *sym; - Elf_Sym *near = NULL; - Elf_Addr distance = ~0; - - for (sym = elf->symtab_start; sym < elf->symtab_stop; sym++) { - const char *symsec; - - if (is_shndx_special(sym->st_shndx)) - continue; - symsec = sec_name(elf, get_secindex(elf, sym)); - if (strcmp(symsec, sec) != 0) - continue; - if (!is_valid_name(elf, sym)) - continue; - if (sym->st_value <= addr) { - if ((addr - sym->st_value) < distance) { - distance = addr - sym->st_value; - near = sym; - } else if ((addr - sym->st_value) == distance) { - near = sym; - } - } - } - return near; -} - -/* - * Convert a section name to the function/data attribute - * .init.text => __init - * .memexitconst => __memconst - * etc. - * - * The memory of returned value has been allocated on a heap. The user of this - * method should free it after usage. -*/ -static char *sec2annotation(const char *s) -{ - if (match(s, init_exit_sections)) { - char *p = NOFAIL(malloc(20)); - char *r = p; - - *p++ = '_'; - *p++ = '_'; - if (*s == '.') - s++; - while (*s && *s != '.') - *p++ = *s++; - *p = '\0'; - if (*s == '.') - s++; - if (strstr(s, "rodata") != NULL) - strcat(p, "const "); - else if (strstr(s, "data") != NULL) - strcat(p, "data "); - else - strcat(p, " "); - return r; - } else { - return NOFAIL(strdup("")); - } -} - -static int is_function(Elf_Sym *sym) -{ - if (sym) - return ELF_ST_TYPE(sym->st_info) == STT_FUNC; - else - return -1; -} - -static void print_section_list(const char * const list[20]) -{ - const char *const *s = list; - - while (*s) { - fprintf(stderr, "%s", *s); - s++; - if (*s) - fprintf(stderr, ", "); - } - fprintf(stderr, "\n"); -} - -static inline void get_pretty_name(int is_func, const char** name, const char** name_p) -{ - switch (is_func) { - case 0: *name = "variable"; *name_p = ""; break; - case 1: *name = "function"; *name_p = "()"; break; - default: *name = "(unknown reference)"; *name_p = ""; break; - } -} - -/* - * Print a warning about a section mismatch. - * Try to find symbols near it so user can find it. - * Check whitelist before warning - it may be a false positive. - */ -static void report_sec_mismatch(const char *modname, - const struct sectioncheck *mismatch, - const char *fromsec, - unsigned long long fromaddr, - const char *fromsym, - int from_is_func, - const char *tosec, const char *tosym, - int to_is_func) -{ - const char *from, *from_p; - const char *to, *to_p; - char *prl_from; - char *prl_to; - - sec_mismatch_count++; - - get_pretty_name(from_is_func, &from, &from_p); - get_pretty_name(to_is_func, &to, &to_p); - - warn("%s(%s+0x%llx): Section mismatch in reference from the %s %s%s " - "to the %s %s:%s%s\n", - modname, fromsec, fromaddr, from, fromsym, from_p, to, tosec, - tosym, to_p); - - switch (mismatch->mismatch) { - case TEXT_TO_ANY_INIT: - prl_from = sec2annotation(fromsec); - prl_to = sec2annotation(tosec); - fprintf(stderr, - "The function %s%s() references\n" - "the %s %s%s%s.\n" - "This is often because %s lacks a %s\n" - "annotation or the annotation of %s is wrong.\n", - prl_from, fromsym, - to, prl_to, tosym, to_p, - fromsym, prl_to, tosym); - free(prl_from); - free(prl_to); - break; - case DATA_TO_ANY_INIT: { - prl_to = sec2annotation(tosec); - fprintf(stderr, - "The variable %s references\n" - "the %s %s%s%s\n" - "If the reference is valid then annotate the\n" - "variable with __init* or __refdata (see linux/init.h) " - "or name the variable:\n", - fromsym, to, prl_to, tosym, to_p); - print_section_list(mismatch->symbol_white_list); - free(prl_to); - break; - } - case TEXT_TO_ANY_EXIT: - prl_to = sec2annotation(tosec); - fprintf(stderr, - "The function %s() references a %s in an exit section.\n" - "Often the %s %s%s has valid usage outside the exit section\n" - "and the fix is to remove the %sannotation of %s.\n", - fromsym, to, to, tosym, to_p, prl_to, tosym); - free(prl_to); - break; - case DATA_TO_ANY_EXIT: { - prl_to = sec2annotation(tosec); - fprintf(stderr, - "The variable %s references\n" - "the %s %s%s%s\n" - "If the reference is valid then annotate the\n" - "variable with __exit* (see linux/init.h) or " - "name the variable:\n", - fromsym, to, prl_to, tosym, to_p); - print_section_list(mismatch->symbol_white_list); - free(prl_to); - break; - } - case XXXINIT_TO_SOME_INIT: - case XXXEXIT_TO_SOME_EXIT: - prl_from = sec2annotation(fromsec); - prl_to = sec2annotation(tosec); - fprintf(stderr, - "The %s %s%s%s references\n" - "a %s %s%s%s.\n" - "If %s is only used by %s then\n" - "annotate %s with a matching annotation.\n", - from, prl_from, fromsym, from_p, - to, prl_to, tosym, to_p, - tosym, fromsym, tosym); - free(prl_from); - free(prl_to); - break; - case ANY_INIT_TO_ANY_EXIT: - prl_from = sec2annotation(fromsec); - prl_to = sec2annotation(tosec); - fprintf(stderr, - "The %s %s%s%s references\n" - "a %s %s%s%s.\n" - "This is often seen when error handling " - "in the init function\n" - "uses functionality in the exit path.\n" - "The fix is often to remove the %sannotation of\n" - "%s%s so it may be used outside an exit section.\n", - from, prl_from, fromsym, from_p, - to, prl_to, tosym, to_p, - prl_to, tosym, to_p); - free(prl_from); - free(prl_to); - break; - case ANY_EXIT_TO_ANY_INIT: - prl_from = sec2annotation(fromsec); - prl_to = sec2annotation(tosec); - fprintf(stderr, - "The %s %s%s%s references\n" - "a %s %s%s%s.\n" - "This is often seen when error handling " - "in the exit function\n" - "uses functionality in the init path.\n" - "The fix is often to remove the %sannotation of\n" - "%s%s so it may be used outside an init section.\n", - from, prl_from, fromsym, from_p, - to, prl_to, tosym, to_p, - prl_to, tosym, to_p); - free(prl_from); - free(prl_to); - break; - case EXPORT_TO_INIT_EXIT: - prl_to = sec2annotation(tosec); - fprintf(stderr, - "The symbol %s is exported and annotated %s\n" - "Fix this by removing the %sannotation of %s " - "or drop the export.\n", - tosym, prl_to, prl_to, tosym); - free(prl_to); - break; - case EXTABLE_TO_NON_TEXT: - fatal("There's a special handler for this mismatch type, " - "we should never get here."); - break; - } - fprintf(stderr, "\n"); -} - -static void default_mismatch_handler(const char *modname, struct elf_info *elf, - const struct sectioncheck* const mismatch, - Elf_Rela *r, Elf_Sym *sym, const char *fromsec) -{ - const char *tosec; - Elf_Sym *to; - Elf_Sym *from; - const char *tosym; - const char *fromsym; - - from = find_elf_symbol2(elf, r->r_offset, fromsec); - fromsym = sym_name(elf, from); - - if (strstarts(fromsym, "reference___initcall")) - return; - - tosec = sec_name(elf, get_secindex(elf, sym)); - to = find_elf_symbol(elf, r->r_addend, sym); - tosym = sym_name(elf, to); - - /* check whitelist - we may ignore it */ - if (secref_whitelist(mismatch, - fromsec, fromsym, tosec, tosym)) { - report_sec_mismatch(modname, mismatch, - fromsec, r->r_offset, fromsym, - is_function(from), tosec, tosym, - is_function(to)); - } -} - -static int is_executable_section(struct elf_info* elf, unsigned int section_index) -{ - if (section_index > elf->num_sections) - fatal("section_index is outside elf->num_sections!\n"); - - return ((elf->sechdrs[section_index].sh_flags & SHF_EXECINSTR) == SHF_EXECINSTR); -} - -/* - * We rely on a gross hack in section_rel[a]() calling find_extable_entry_size() - * to know the sizeof(struct exception_table_entry) for the target architecture. - */ -static unsigned int extable_entry_size = 0; -static void find_extable_entry_size(const char* const sec, const Elf_Rela* r) -{ - /* - * If we're currently checking the second relocation within __ex_table, - * that relocation offset tells us the offsetof(struct - * exception_table_entry, fixup) which is equal to sizeof(struct - * exception_table_entry) divided by two. We use that to our advantage - * since there's no portable way to get that size as every architecture - * seems to go with different sized types. Not pretty but better than - * hard-coding the size for every architecture.. - */ - if (!extable_entry_size) - extable_entry_size = r->r_offset * 2; -} - -static inline bool is_extable_fault_address(Elf_Rela *r) -{ - /* - * extable_entry_size is only discovered after we've handled the - * _second_ relocation in __ex_table, so only abort when we're not - * handling the first reloc and extable_entry_size is zero. - */ - if (r->r_offset && extable_entry_size == 0) - fatal("extable_entry size hasn't been discovered!\n"); - - return ((r->r_offset == 0) || - (r->r_offset % extable_entry_size == 0)); -} - -#define is_second_extable_reloc(Start, Cur, Sec) \ - (((Cur) == (Start) + 1) && (strcmp("__ex_table", (Sec)) == 0)) - -static void report_extable_warnings(const char* modname, struct elf_info* elf, - const struct sectioncheck* const mismatch, - Elf_Rela* r, Elf_Sym* sym, - const char* fromsec, const char* tosec) -{ - Elf_Sym* fromsym = find_elf_symbol2(elf, r->r_offset, fromsec); - const char* fromsym_name = sym_name(elf, fromsym); - Elf_Sym* tosym = find_elf_symbol(elf, r->r_addend, sym); - const char* tosym_name = sym_name(elf, tosym); - const char* from_pretty_name; - const char* from_pretty_name_p; - const char* to_pretty_name; - const char* to_pretty_name_p; - - get_pretty_name(is_function(fromsym), - &from_pretty_name, &from_pretty_name_p); - get_pretty_name(is_function(tosym), - &to_pretty_name, &to_pretty_name_p); - - warn("%s(%s+0x%lx): Section mismatch in reference" - " from the %s %s%s to the %s %s:%s%s\n", - modname, fromsec, (long)r->r_offset, from_pretty_name, - fromsym_name, from_pretty_name_p, - to_pretty_name, tosec, tosym_name, to_pretty_name_p); - - if (!match(tosec, mismatch->bad_tosec) && - is_executable_section(elf, get_secindex(elf, sym))) - fprintf(stderr, - "The relocation at %s+0x%lx references\n" - "section \"%s\" which is not in the list of\n" - "authorized sections. If you're adding a new section\n" - "and/or if this reference is valid, add \"%s\" to the\n" - "list of authorized sections to jump to on fault.\n" - "This can be achieved by adding \"%s\" to \n" - "OTHER_TEXT_SECTIONS in scripts/mod/modpost.c.\n", - fromsec, (long)r->r_offset, tosec, tosec, tosec); -} - -static void extable_mismatch_handler(const char* modname, struct elf_info *elf, - const struct sectioncheck* const mismatch, - Elf_Rela* r, Elf_Sym* sym, - const char *fromsec) -{ - const char* tosec = sec_name(elf, get_secindex(elf, sym)); - - sec_mismatch_count++; - - report_extable_warnings(modname, elf, mismatch, r, sym, fromsec, tosec); - - if (match(tosec, mismatch->bad_tosec)) - fatal("The relocation at %s+0x%lx references\n" - "section \"%s\" which is black-listed.\n" - "Something is seriously wrong and should be fixed.\n" - "You might get more information about where this is\n" - "coming from by using scripts/check_extable.sh %s\n", - fromsec, (long)r->r_offset, tosec, modname); - else if (!is_executable_section(elf, get_secindex(elf, sym))) { - if (is_extable_fault_address(r)) - fatal("The relocation at %s+0x%lx references\n" - "section \"%s\" which is not executable, IOW\n" - "it is not possible for the kernel to fault\n" - "at that address. Something is seriously wrong\n" - "and should be fixed.\n", - fromsec, (long)r->r_offset, tosec); - else - fatal("The relocation at %s+0x%lx references\n" - "section \"%s\" which is not executable, IOW\n" - "the kernel will fault if it ever tries to\n" - "jump to it. Something is seriously wrong\n" - "and should be fixed.\n", - fromsec, (long)r->r_offset, tosec); - } -} - -static void check_section_mismatch(const char *modname, struct elf_info *elf, - Elf_Rela *r, Elf_Sym *sym, const char *fromsec) -{ - const char *tosec = sec_name(elf, get_secindex(elf, sym)); - const struct sectioncheck *mismatch = section_mismatch(fromsec, tosec); - - if (mismatch) { - if (mismatch->handler) - mismatch->handler(modname, elf, mismatch, - r, sym, fromsec); - else - default_mismatch_handler(modname, elf, mismatch, - r, sym, fromsec); - } -} - -static unsigned int *reloc_location(struct elf_info *elf, - Elf_Shdr *sechdr, Elf_Rela *r) -{ - return sym_get_data_by_offset(elf, sechdr->sh_info, r->r_offset); -} - -static int addend_386_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r) -{ - unsigned int r_typ = ELF_R_TYPE(r->r_info); - unsigned int *location = reloc_location(elf, sechdr, r); - - switch (r_typ) { - case R_386_32: - r->r_addend = TO_NATIVE(*location); - break; - case R_386_PC32: - r->r_addend = TO_NATIVE(*location) + 4; - /* For CONFIG_RELOCATABLE=y */ - if (elf->hdr->e_type == ET_EXEC) - r->r_addend += r->r_offset; - break; - } - return 0; -} - -#ifndef R_ARM_CALL -#define R_ARM_CALL 28 -#endif -#ifndef R_ARM_JUMP24 -#define R_ARM_JUMP24 29 -#endif - -#ifndef R_ARM_THM_CALL -#define R_ARM_THM_CALL 10 -#endif -#ifndef R_ARM_THM_JUMP24 -#define R_ARM_THM_JUMP24 30 -#endif -#ifndef R_ARM_THM_JUMP19 -#define R_ARM_THM_JUMP19 51 -#endif - -static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r) -{ - unsigned int r_typ = ELF_R_TYPE(r->r_info); - - switch (r_typ) { - case R_ARM_ABS32: - /* From ARM ABI: (S + A) | T */ - r->r_addend = (int)(long) - (elf->symtab_start + ELF_R_SYM(r->r_info)); - break; - case R_ARM_PC24: - case R_ARM_CALL: - case R_ARM_JUMP24: - case R_ARM_THM_CALL: - case R_ARM_THM_JUMP24: - case R_ARM_THM_JUMP19: - /* From ARM ABI: ((S + A) | T) - P */ - r->r_addend = (int)(long)(elf->hdr + - sechdr->sh_offset + - (r->r_offset - sechdr->sh_addr)); - break; - default: - return 1; - } - return 0; -} - -static int addend_mips_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r) -{ - unsigned int r_typ = ELF_R_TYPE(r->r_info); - unsigned int *location = reloc_location(elf, sechdr, r); - unsigned int inst; - - if (r_typ == R_MIPS_HI16) - return 1; /* skip this */ - inst = TO_NATIVE(*location); - switch (r_typ) { - case R_MIPS_LO16: - r->r_addend = inst & 0xffff; - break; - case R_MIPS_26: - r->r_addend = (inst & 0x03ffffff) << 2; - break; - case R_MIPS_32: - r->r_addend = inst; - break; - } - return 0; -} - -#ifndef EM_RISCV -#define EM_RISCV 243 -#endif - -#ifndef R_RISCV_SUB32 -#define R_RISCV_SUB32 39 -#endif - -static void section_rela(const char *modname, struct elf_info *elf, - Elf_Shdr *sechdr) -{ - Elf_Sym *sym; - Elf_Rela *rela; - Elf_Rela r; - unsigned int r_sym; - const char *fromsec; - - Elf_Rela *start = (void *)elf->hdr + sechdr->sh_offset; - Elf_Rela *stop = (void *)start + sechdr->sh_size; - - fromsec = sech_name(elf, sechdr); - fromsec += strlen(".rela"); - /* if from section (name) is know good then skip it */ - if (match(fromsec, section_white_list)) - return; - - for (rela = start; rela < stop; rela++) { - r.r_offset = TO_NATIVE(rela->r_offset); -#if KERNEL_ELFCLASS == ELFCLASS64 - if (elf->hdr->e_machine == EM_MIPS) { - unsigned int r_typ; - r_sym = ELF64_MIPS_R_SYM(rela->r_info); - r_sym = TO_NATIVE(r_sym); - r_typ = ELF64_MIPS_R_TYPE(rela->r_info); - r.r_info = ELF64_R_INFO(r_sym, r_typ); - } else { - r.r_info = TO_NATIVE(rela->r_info); - r_sym = ELF_R_SYM(r.r_info); - } -#else - r.r_info = TO_NATIVE(rela->r_info); - r_sym = ELF_R_SYM(r.r_info); -#endif - r.r_addend = TO_NATIVE(rela->r_addend); - switch (elf->hdr->e_machine) { - case EM_RISCV: - if (!strcmp("__ex_table", fromsec) && - ELF_R_TYPE(r.r_info) == R_RISCV_SUB32) - continue; - break; - } - sym = elf->symtab_start + r_sym; - /* Skip special sections */ - if (is_shndx_special(sym->st_shndx)) - continue; - if (is_second_extable_reloc(start, rela, fromsec)) - find_extable_entry_size(fromsec, &r); - check_section_mismatch(modname, elf, &r, sym, fromsec); - } -} - -static void section_rel(const char *modname, struct elf_info *elf, - Elf_Shdr *sechdr) -{ - Elf_Sym *sym; - Elf_Rel *rel; - Elf_Rela r; - unsigned int r_sym; - const char *fromsec; - - Elf_Rel *start = (void *)elf->hdr + sechdr->sh_offset; - Elf_Rel *stop = (void *)start + sechdr->sh_size; - - fromsec = sech_name(elf, sechdr); - fromsec += strlen(".rel"); - /* if from section (name) is know good then skip it */ - if (match(fromsec, section_white_list)) - return; - - for (rel = start; rel < stop; rel++) { - r.r_offset = TO_NATIVE(rel->r_offset); -#if KERNEL_ELFCLASS == ELFCLASS64 - if (elf->hdr->e_machine == EM_MIPS) { - unsigned int r_typ; - r_sym = ELF64_MIPS_R_SYM(rel->r_info); - r_sym = TO_NATIVE(r_sym); - r_typ = ELF64_MIPS_R_TYPE(rel->r_info); - r.r_info = ELF64_R_INFO(r_sym, r_typ); - } else { - r.r_info = TO_NATIVE(rel->r_info); - r_sym = ELF_R_SYM(r.r_info); - } -#else - r.r_info = TO_NATIVE(rel->r_info); - r_sym = ELF_R_SYM(r.r_info); -#endif - r.r_addend = 0; - switch (elf->hdr->e_machine) { - case EM_386: - if (addend_386_rel(elf, sechdr, &r)) - continue; - break; - case EM_ARM: - if (addend_arm_rel(elf, sechdr, &r)) - continue; - break; - case EM_MIPS: - if (addend_mips_rel(elf, sechdr, &r)) - continue; - break; - } - sym = elf->symtab_start + r_sym; - /* Skip special sections */ - if (is_shndx_special(sym->st_shndx)) - continue; - if (is_second_extable_reloc(start, rel, fromsec)) - find_extable_entry_size(fromsec, &r); - check_section_mismatch(modname, elf, &r, sym, fromsec); - } -} - -/** - * A module includes a number of sections that are discarded - * either when loaded or when used as built-in. - * For loaded modules all functions marked __init and all data - * marked __initdata will be discarded when the module has been initialized. - * Likewise for modules used built-in the sections marked __exit - * are discarded because __exit marked function are supposed to be called - * only when a module is unloaded which never happens for built-in modules. - * The check_sec_ref() function traverses all relocation records - * to find all references to a section that reference a section that will - * be discarded and warns about it. - **/ -static void check_sec_ref(struct module *mod, const char *modname, - 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(modname, elf, &elf->sechdrs[i]); - /* We want to process only relocation sections and not .init */ - if (sechdrs[i].sh_type == SHT_RELA) - section_rela(modname, elf, &elf->sechdrs[i]); - else if (sechdrs[i].sh_type == SHT_REL) - section_rel(modname, elf, &elf->sechdrs[i]); - } -} - static char *remove_dot(char *s) { size_t n = strcspn(s, "."); diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h index d9daeff07b83..ffa4512e992f 100644 --- a/scripts/mod/modpost.h +++ b/scripts/mod/modpost.h @@ -52,28 +52,6 @@ #define ELF_R_TYPE ELF64_R_TYPE #endif -/* The 64-bit MIPS ELF ABI uses an unusual reloc format. */ -typedef struct -{ - Elf32_Word r_sym; /* Symbol index */ - unsigned char r_ssym; /* Special symbol for 2nd relocation */ - unsigned char r_type3; /* 3rd relocation type */ - unsigned char r_type2; /* 2nd relocation type */ - unsigned char r_type1; /* 1st relocation type */ -} _Elf64_Mips_R_Info; - -typedef union -{ - Elf64_Xword r_info_number; - _Elf64_Mips_R_Info r_info_fields; -} _Elf64_Mips_R_Info_union; - -#define ELF64_MIPS_R_SYM(i) \ - ((__extension__ (_Elf64_Mips_R_Info_union)(i)).r_info_fields.r_sym) - -#define ELF64_MIPS_R_TYPE(i) \ - ((__extension__ (_Elf64_Mips_R_Info_union)(i)).r_info_fields.r_type1) - #if KERNEL_ELFDATA != HOST_ELFDATA static inline void __endian(const void *src, void *dest, unsigned int size) @@ -96,6 +74,7 @@ static inline void __endian(const void *src, void *dest, unsigned int size) #endif +#define strstarts(str, prefix) (strncmp(str, prefix, strlen(prefix)) == 0) #define NOFAIL(ptr) do_nofail((ptr), #ptr) void *do_nofail(void *ptr, const char *expr); @@ -173,6 +152,10 @@ static inline unsigned int get_secindex(const struct elf_info *info, return info->symtab_shndx_start[sym - info->symtab_start]; } +/* section-check.c */ +void check_sec_ref(struct module *mod, const char *modname, + struct elf_info *elf); + /* file2alias.c */ void handle_moddevtable(struct module *mod, struct elf_info *info, Elf_Sym *sym, const char *symname); @@ -182,6 +165,13 @@ void add_moddevtable(struct buffer *buf, struct module *mod); void get_src_version(const char *modname, char sum[], unsigned sumlen); /* from modpost.c */ +extern int sec_mismatch_count; + +void *sym_get_data_by_offset(const struct elf_info *info, + unsigned int secindex, unsigned long offset); +const char *sech_name(const struct elf_info *info, Elf_Shdr *sechdr); +const char *sec_name(const struct elf_info *info, int secindex); + char *read_text_file(const char *filename); char *get_line(char **stringp); diff --git a/scripts/mod/section-check.c b/scripts/mod/section-check.c new file mode 100644 index 000000000000..0412658751a1 --- /dev/null +++ b/scripts/mod/section-check.c @@ -0,0 +1,1222 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#include "modpost.h" + +/* The 64-bit MIPS ELF ABI uses an unusual reloc format. */ +typedef struct { + Elf32_Word r_sym; /* Symbol index */ + unsigned char r_ssym; /* Special symbol for 2nd relocation */ + unsigned char r_type3; /* 3rd relocation type */ + unsigned char r_type2; /* 2nd relocation type */ + unsigned char r_type1; /* 1st relocation type */ +} _Elf64_Mips_R_Info; + +typedef union { + Elf64_Xword r_info_number; + _Elf64_Mips_R_Info r_info_fields; +} _Elf64_Mips_R_Info_union; + +#define ELF64_MIPS_R_SYM(i) \ + ((__extension__ (_Elf64_Mips_R_Info_union)(i)).r_info_fields.r_sym) + +#define ELF64_MIPS_R_TYPE(i) \ + ((__extension__ (_Elf64_Mips_R_Info_union)(i)).r_info_fields.r_type1) + +/* Test if string s ends in string sub. Return 0 if match. */ +static int strrcmp(const char *s, const char *sub) +{ + int slen, sublen; + + if (!s || !sub) + return 1; + + slen = strlen(s); + sublen = strlen(sub); + + if ((slen == 0) || (sublen == 0)) + return 1; + + if (sublen > slen) + return 1; + + return memcmp(s + slen - sublen, sub, sublen); +} + +static const char *sym_name(struct elf_info *elf, Elf_Sym *sym) +{ + if (sym) + return elf->strtab + sym->st_name; + else + return "(unknown)"; +} + +/* + * The pattern is an array of simple patterns. + * "foo" will match an exact string equal to "foo" + * "*foo" will match a string that ends with "foo" + * "foo*" will match a string that begins with "foo" + * "*foo*" will match a string that contains "foo" + */ +static int match(const char *sym, const char * const pat[]) +{ + const char *p; + + while (*pat) { + const char *endp; + + p = *pat++; + endp = p + strlen(p) - 1; + + /* "*foo*" */ + if (*p == '*' && *endp == '*') { + char *bare = NOFAIL(strndup(p + 1, strlen(p) - 2)); + char *here = strstr(sym, bare); + + free(bare); + if (here != NULL) + return 1; + } + /* "*foo" */ + else if (*p == '*') { + if (strrcmp(sym, p + 1) == 0) + return 1; + } + /* "foo*" */ + else if (*endp == '*') { + if (strncmp(sym, p, strlen(p) - 1) == 0) + return 1; + } + /* no wildcards */ + else { + if (strcmp(p, sym) == 0) + return 1; + } + } + /* no match */ + return 0; +} + +/* sections that we do not want to do full section mismatch check on */ +static const char *const section_white_list[] = { + ".comment*", + ".debug*", + ".cranges", /* sh64 */ + ".zdebug*", /* Compressed debug sections. */ + ".GCC.command.line", /* record-gcc-switches */ + ".mdebug*", /* alpha, score, mips etc. */ + ".pdr", /* alpha, score, mips etc. */ + ".stab*", + ".note*", + ".got*", + ".toc*", + ".xt.prop", /* xtensa */ + ".xt.lit", /* xtensa */ + ".arcextmap*", /* arc */ + ".gnu.linkonce.arcext*", /* arc : modules */ + ".cmem*", /* EZchip */ + ".fmt_slot*", /* EZchip */ + ".gnu.lto*", + ".discard.*", + NULL +}; + +/* + * This is used to find sections missing the SHF_ALLOC flag. + * The cause of this is often a section specified in assembler + * without "ax" / "aw". + */ +static void check_section(const char *modname, struct elf_info *elf, + Elf_Shdr *sechdr) +{ + const char *sec = sech_name(elf, sechdr); + + if (sechdr->sh_type == SHT_PROGBITS && + !(sechdr->sh_flags & SHF_ALLOC) && + !match(sec, section_white_list)) { + warn("%s (%s): unexpected non-allocatable section.\n" + "Did you forget to use \"ax\"/\"aw\" in a .S file?\n" + "Note that for example <linux/init.h> contains\n" + "section definitions for use in .S files.\n\n", + modname, sec); + } +} + +#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" +#define ALL_EXIT_TEXT_SECTIONS \ + ".exit.text", ".memexit.text" + +#define ALL_PCI_INIT_SECTIONS \ + ".pci_fixup_early", ".pci_fixup_header", ".pci_fixup_final", \ + ".pci_fixup_enable", ".pci_fixup_resume", \ + ".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 DATA_SECTIONS ".data", ".data.rel" +#define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \ + ".kprobes.text", ".cpuidle.text", ".noinstr.text" +#define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \ + ".fixup", ".entry.text", ".exception.text", ".text.*", \ + ".coldtext", ".softirqentry.text" + +#define INIT_SECTIONS ".init.*" +#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 + +/* init data sections */ +static const char *const init_data_sections[] = { + ALL_INIT_DATA_SECTIONS, NULL +}; + +/* all init sections */ +static const char *const init_sections[] = { + ALL_INIT_SECTIONS, NULL +}; + +/* All init and exit sections (code + data) */ +static const char *const init_exit_sections[] = { + ALL_INIT_SECTIONS, ALL_EXIT_SECTIONS, NULL +}; + +/* all text sections */ +static const char *const text_sections[] = { + ALL_TEXT_SECTIONS, NULL +}; + +/* data section */ +static const char *const data_sections[] = { + DATA_SECTIONS, NULL +}; + +/* symbols in .data that may refer to init/exit sections */ +#define DEFAULT_SYMBOL_WHITE_LIST \ + "*driver", \ + "*_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" + +static const char *const head_sections[] = { + ".head.text*", NULL +}; +static const char *const linker_symbols[] = { + "__init_begin", "_sinittext", "_einittext", NULL +}; +static const char *const optim_symbols[] = { + "*.constprop.*", NULL +}; + +enum mismatch { + TEXT_TO_ANY_INIT, + DATA_TO_ANY_INIT, + TEXT_TO_ANY_EXIT, + DATA_TO_ANY_EXIT, + XXXINIT_TO_SOME_INIT, + XXXEXIT_TO_SOME_EXIT, + ANY_INIT_TO_ANY_EXIT, + ANY_EXIT_TO_ANY_INIT, + EXPORT_TO_INIT_EXIT, + EXTABLE_TO_NON_TEXT, +}; + +/* + * Describe how to match sections on different criteria: + * + * @fromsec: Array of sections to be matched. + * + * @bad_tosec: Relocations applied to a section in @fromsec to a section in + * this array is forbidden (black-list). Can be empty. + * + * @good_tosec: Relocations applied to a section in @fromsec must be + * targeting sections in this array (white-list). Can be empty. + * + * @mismatch: Type of mismatch. + * + * @symbol_white_list: Do not match a relocation to a symbol in this list + * even if it is targeting a section in @bad_to_sec. + * + * @handler: Specific handler to call when a match is found. If NULL, + * default_mismatch_handler() will be called. + */ +struct sectioncheck { + const char *fromsec[20]; + const char *bad_tosec[20]; + const char *good_tosec[20]; + enum mismatch mismatch; + const char *symbol_white_list[20]; + void (*handler)(const char *modname, struct elf_info *elf, + const struct sectioncheck *mismatch, + Elf_Rela *r, Elf_Sym *sym, const char *fromsec); +}; + +static void extable_mismatch_handler(const char *modname, struct elf_info *elf, + const struct sectioncheck *mismatch, + Elf_Rela *r, Elf_Sym *sym, + const char *fromsec); + +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, + .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, + }, + { + .fromsec = { DATA_SECTIONS, NULL }, + .bad_tosec = { ALL_XXXINIT_SECTIONS, NULL }, + .mismatch = DATA_TO_ANY_INIT, + .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, + }, + { + .fromsec = { DATA_SECTIONS, NULL }, + .bad_tosec = { INIT_SECTIONS, NULL }, + .mismatch = DATA_TO_ANY_INIT, + .symbol_white_list = { + "*_template", "*_timer", "*_sht", "*_ops", + "*_probe", "*_probe_one", "*_console", NULL + }, + }, + { + .fromsec = { TEXT_SECTIONS, NULL }, + .bad_tosec = { ALL_EXIT_SECTIONS, NULL }, + .mismatch = TEXT_TO_ANY_EXIT, + .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, + }, + { + .fromsec = { DATA_SECTIONS, NULL }, + .bad_tosec = { ALL_EXIT_SECTIONS, NULL }, + .mismatch = DATA_TO_ANY_EXIT, + .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, + }, + /* Do not reference init code/data from meminit code/data */ + { + .fromsec = { ALL_XXXINIT_SECTIONS, NULL }, + .bad_tosec = { INIT_SECTIONS, NULL }, + .mismatch = XXXINIT_TO_SOME_INIT, + .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, + }, + /* 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, + .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, + }, + /* Do not use exit code/data from init code */ + { + .fromsec = { ALL_INIT_SECTIONS, NULL }, + .bad_tosec = { ALL_EXIT_SECTIONS, NULL }, + .mismatch = ANY_INIT_TO_ANY_EXIT, + .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, + }, + /* Do not use init code/data from exit code */ + { + .fromsec = { ALL_EXIT_SECTIONS, NULL }, + .bad_tosec = { ALL_INIT_SECTIONS, NULL }, + .mismatch = ANY_EXIT_TO_ANY_INIT, + .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, + }, + { + .fromsec = { ALL_PCI_INIT_SECTIONS, NULL }, + .bad_tosec = { INIT_SECTIONS, NULL }, + .mismatch = ANY_INIT_TO_ANY_EXIT, + .symbol_white_list = { NULL }, + }, + /* Do not export init/exit functions or data */ + { + .fromsec = { "__ksymtab*", NULL }, + .bad_tosec = { INIT_SECTIONS, EXIT_SECTIONS, NULL }, + .mismatch = EXPORT_TO_INIT_EXIT, + .symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL }, + }, + { + .fromsec = { "__ex_table", NULL }, + /* + * If you're adding any new black-listed sections in here, + * consider adding a special 'printer' for them in + * scripts/check_extable. + */ + .bad_tosec = { ".altinstr_replacement", NULL }, + .good_tosec = {ALL_TEXT_SECTIONS, NULL}, + .mismatch = EXTABLE_TO_NON_TEXT, + .handler = extable_mismatch_handler, + } +}; + +static const struct sectioncheck *section_mismatch(const char *fromsec, + const char *tosec) +{ + int i; + int elems = sizeof(sectioncheck) / sizeof(struct sectioncheck); + const struct sectioncheck *check = §ioncheck[0]; + + /* + * The target section could be the SHT_NUL section when we're + * handling relocations to un-resolved symbols, trying to match it + * doesn't make much sense and causes build failures on parisc + * architectures. + */ + if (*tosec == '\0') + return NULL; + + for (i = 0; i < elems; i++) { + if (match(fromsec, check->fromsec)) { + if (check->bad_tosec[0] && match(tosec, check->bad_tosec)) + return check; + if (check->good_tosec[0] && !match(tosec, check->good_tosec)) + return check; + } + check++; + } + return NULL; +} + +/* + * Whitelist to allow certain references to pass with no warning. + * + * Pattern 1: + * If a module parameter is declared __initdata and permissions=0 + * then this is legal despite the warning generated. + * We cannot see value of permissions here, so just ignore + * this pattern. + * The pattern is identified by: + * tosec = .init.data + * fromsec = .data* + * atsym =__param* + * + * Pattern 1a: + * module_param_call() ops can refer to __init set function if permissions=0 + * The pattern is identified by: + * tosec = .init.text + * fromsec = .data* + * atsym = __param_ops_* + * + * Pattern 2: + * Many drivers utilise a *driver container with references to + * add, remove, probe functions etc. + * the pattern is identified by: + * tosec = init or exit section + * fromsec = data section + * atsym = *driver, *_template, *_sht, *_ops, *_probe, + * *probe_one, *_console, *_timer + * + * Pattern 3: + * Whitelist all references from .head.text to any init section + * + * Pattern 4: + * Some symbols belong to init section but still it is ok to reference + * these from non-init sections as these symbols don't have any memory + * allocated for them and symbol address and value are same. So even + * if init section is freed, its ok to reference those symbols. + * For ex. symbols marking the init section boundaries. + * This pattern is identified by + * refsymname = __init_begin, _sinittext, _einittext + * + * Pattern 5: + * GCC may optimize static inlines when fed constant arg(s) resulting + * in functions like cpumask_empty() -- generating an associated symbol + * cpumask_empty.constprop.3 that appears in the audit. If the const that + * is passed in comes from __init, like say nmi_ipi_mask, we get a + * meaningless section warning. May need to add isra symbols too... + * This pattern is identified by + * tosec = init section + * fromsec = text section + * refsymname = *.constprop.* + * + * Pattern 6: + * Hide section mismatch warnings for ELF local symbols. The goal + * is to eliminate false positive modpost warnings caused by + * compiler-generated ELF local symbol names such as ".LANCHOR1". + * Autogenerated symbol names bypass modpost's "Pattern 2" + * whitelisting, which relies on pattern-matching against symbol + * names to work. (One situation where gcc can autogenerate ELF + * local symbols is when "-fsection-anchors" is used.) + */ +static int secref_whitelist(const struct sectioncheck *mismatch, + const char *fromsec, const char *fromsym, + const char *tosec, const char *tosym) +{ + /* Check for pattern 1 */ + if (match(tosec, init_data_sections) && + match(fromsec, data_sections) && + strstarts(fromsym, "__param")) + return 0; + + /* Check for pattern 1a */ + if (strcmp(tosec, ".init.text") == 0 && + match(fromsec, data_sections) && + strstarts(fromsym, "__param_ops_")) + return 0; + + /* Check for pattern 2 */ + if (match(tosec, init_exit_sections) && + match(fromsec, data_sections) && + match(fromsym, mismatch->symbol_white_list)) + return 0; + + /* Check for pattern 3 */ + if (match(fromsec, head_sections) && + match(tosec, init_sections)) + return 0; + + /* Check for pattern 4 */ + if (match(tosym, linker_symbols)) + return 0; + + /* Check for pattern 5 */ + if (match(fromsec, text_sections) && + match(tosec, init_sections) && + match(fromsym, optim_symbols)) + return 0; + + /* Check for pattern 6 */ + if (strstarts(fromsym, ".L")) + return 0; + + return 1; +} + +static inline int is_arm_mapping_symbol(const char *str) +{ + return str[0] == '$' && strchr("axtd", str[1]) + && (str[2] == '\0' || str[2] == '.'); +} + +/* + * If there's no name there, ignore it; likewise, ignore it if it's + * one of the magic symbols emitted used by current ARM 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_arm_mapping_symbol(name); +} + +/* + * Find symbol based on relocation record info. + * In some cases the symbol supplied is a valid symbol so + * return refsym. If st_name != 0 we assume this is a valid symbol. + * In other cases the symbol needs to be looked up in the symbol table + * based on section and address. + */ +static Elf_Sym *find_elf_symbol(struct elf_info *elf, Elf64_Sword addr, + Elf_Sym *relsym) +{ + Elf_Sym *sym; + Elf_Sym *near = NULL; + Elf64_Sword distance = 20; + Elf64_Sword d; + unsigned int relsym_secindex; + + if (relsym->st_name != 0) + return relsym; + + relsym_secindex = get_secindex(elf, relsym); + for (sym = elf->symtab_start; sym < elf->symtab_stop; sym++) { + if (get_secindex(elf, sym) != relsym_secindex) + continue; + if (ELF_ST_TYPE(sym->st_info) == STT_SECTION) + continue; + if (!is_valid_name(elf, sym)) + continue; + if (sym->st_value == addr) + return sym; + /* Find a symbol nearby - addr are maybe negative */ + d = sym->st_value - addr; + if (d < 0) + d = addr - sym->st_value; + if (d < distance) { + distance = d; + near = sym; + } + } + /* We need a close match */ + if (distance < 20) + return near; + else + return NULL; +} + +/* + * Find symbols before or equal addr and after addr - in the section sec. + * If we find two symbols with equal offset prefer one with a valid name. + * The ELF format may have a better way to detect what type of symbol + * it is, but this works for now. + */ +static Elf_Sym *find_elf_symbol2(struct elf_info *elf, Elf_Addr addr, + const char *sec) +{ + Elf_Sym *sym; + Elf_Sym *near = NULL; + Elf_Addr distance = ~0; + + for (sym = elf->symtab_start; sym < elf->symtab_stop; sym++) { + const char *symsec; + + if (is_shndx_special(sym->st_shndx)) + continue; + symsec = sec_name(elf, get_secindex(elf, sym)); + if (strcmp(symsec, sec) != 0) + continue; + if (!is_valid_name(elf, sym)) + continue; + if (sym->st_value <= addr) { + if ((addr - sym->st_value) < distance) { + distance = addr - sym->st_value; + near = sym; + } else if ((addr - sym->st_value) == distance) { + near = sym; + } + } + } + return near; +} + +/* + * Convert a section name to the function/data attribute + * .init.text => __init + * .memexitconst => __memconst + * etc. + * + * The memory of returned value has been allocated on a heap. The user of this + * method should free it after usage. + */ +static char *sec2annotation(const char *s) +{ + if (match(s, init_exit_sections)) { + char *p = NOFAIL(malloc(20)); + char *r = p; + + *p++ = '_'; + *p++ = '_'; + if (*s == '.') + s++; + while (*s && *s != '.') + *p++ = *s++; + *p = '\0'; + if (*s == '.') + s++; + if (strstr(s, "rodata") != NULL) + strcat(p, "const "); + else if (strstr(s, "data") != NULL) + strcat(p, "data "); + else + strcat(p, " "); + return r; + } else { + return NOFAIL(strdup("")); + } +} + +static int is_function(Elf_Sym *sym) +{ + if (sym) + return ELF_ST_TYPE(sym->st_info) == STT_FUNC; + else + return -1; +} + +static void print_section_list(const char * const list[20]) +{ + const char *const *s = list; + + while (*s) { + fprintf(stderr, "%s", *s); + s++; + if (*s) + fprintf(stderr, ", "); + } + fprintf(stderr, "\n"); +} + +static inline void get_pretty_name(int is_func, const char **name, + const char **name_p) +{ + switch (is_func) { + case 0: + *name = "variable"; + *name_p = ""; + break; + case 1: + *name = "function"; + *name_p = "()"; + break; + default: + *name = "(unknown reference)"; + *name_p = ""; + break; + } +} + +/* + * Print a warning about a section mismatch. + * Try to find symbols near it so user can find it. + * Check whitelist before warning - it may be a false positive. + */ +static void report_sec_mismatch(const char *modname, + const struct sectioncheck *mismatch, + const char *fromsec, + unsigned long long fromaddr, + const char *fromsym, + int from_is_func, + const char *tosec, const char *tosym, + int to_is_func) +{ + const char *from, *from_p; + const char *to, *to_p; + char *prl_from; + char *prl_to; + + sec_mismatch_count++; + + get_pretty_name(from_is_func, &from, &from_p); + get_pretty_name(to_is_func, &to, &to_p); + + warn("%s(%s+0x%llx): Section mismatch in reference from the %s %s%s to the %s %s:%s%s\n", + modname, fromsec, fromaddr, from, fromsym, from_p, to, tosec, + tosym, to_p); + + switch (mismatch->mismatch) { + case TEXT_TO_ANY_INIT: + prl_from = sec2annotation(fromsec); + prl_to = sec2annotation(tosec); + fprintf(stderr, + "The function %s%s() references\n" + "the %s %s%s%s.\n" + "This is often because %s lacks a %s\n" + "annotation or the annotation of %s is wrong.\n", + prl_from, fromsym, + to, prl_to, tosym, to_p, + fromsym, prl_to, tosym); + free(prl_from); + free(prl_to); + break; + case DATA_TO_ANY_INIT: { + prl_to = sec2annotation(tosec); + fprintf(stderr, + "The variable %s references\n" + "the %s %s%s%s\n" + "If the reference is valid then annotate the\n" + "variable with __init* or __refdata (see linux/init.h) or name the variable:\n", + fromsym, to, prl_to, tosym, to_p); + print_section_list(mismatch->symbol_white_list); + free(prl_to); + break; + } + case TEXT_TO_ANY_EXIT: + prl_to = sec2annotation(tosec); + fprintf(stderr, + "The function %s() references a %s in an exit section.\n" + "Often the %s %s%s has valid usage outside the exit section\n" + "and the fix is to remove the %sannotation of %s.\n", + fromsym, to, to, tosym, to_p, prl_to, tosym); + free(prl_to); + break; + case DATA_TO_ANY_EXIT: { + prl_to = sec2annotation(tosec); + fprintf(stderr, + "The variable %s references\n" + "the %s %s%s%s\n" + "If the reference is valid then annotate the\n" + "variable with __exit* (see linux/init.h) or name the variable:\n", + fromsym, to, prl_to, tosym, to_p); + print_section_list(mismatch->symbol_white_list); + free(prl_to); + break; + } + case XXXINIT_TO_SOME_INIT: + case XXXEXIT_TO_SOME_EXIT: + prl_from = sec2annotation(fromsec); + prl_to = sec2annotation(tosec); + fprintf(stderr, + "The %s %s%s%s references\n" + "a %s %s%s%s.\n" + "If %s is only used by %s then\n" + "annotate %s with a matching annotation.\n", + from, prl_from, fromsym, from_p, + to, prl_to, tosym, to_p, + tosym, fromsym, tosym); + free(prl_from); + free(prl_to); + break; + case ANY_INIT_TO_ANY_EXIT: + prl_from = sec2annotation(fromsec); + prl_to = sec2annotation(tosec); + fprintf(stderr, + "The %s %s%s%s references\n" + "a %s %s%s%s.\n" + "This is often seen when error handling in the init function\n" + "uses functionality in the exit path.\n" + "The fix is often to remove the %sannotation of\n" + "%s%s so it may be used outside an exit section.\n", + from, prl_from, fromsym, from_p, + to, prl_to, tosym, to_p, + prl_to, tosym, to_p); + free(prl_from); + free(prl_to); + break; + case ANY_EXIT_TO_ANY_INIT: + prl_from = sec2annotation(fromsec); + prl_to = sec2annotation(tosec); + fprintf(stderr, + "The %s %s%s%s references\n" + "a %s %s%s%s.\n" + "This is often seen when error handling in the exit function\n" + "uses functionality in the init path.\n" + "The fix is often to remove the %sannotation of\n" + "%s%s so it may be used outside an init section.\n", + from, prl_from, fromsym, from_p, + to, prl_to, tosym, to_p, + prl_to, tosym, to_p); + free(prl_from); + free(prl_to); + break; + case EXPORT_TO_INIT_EXIT: + prl_to = sec2annotation(tosec); + fprintf(stderr, + "The symbol %s is exported and annotated %s\n" + "Fix this by removing the %sannotation of %s or drop the export.\n", + tosym, prl_to, prl_to, tosym); + free(prl_to); + break; + case EXTABLE_TO_NON_TEXT: + fatal("There's a special handler for this mismatch type, we should never get here."); + break; + } + fprintf(stderr, "\n"); +} + +static void default_mismatch_handler(const char *modname, struct elf_info *elf, + const struct sectioncheck *mismatch, + Elf_Rela *r, Elf_Sym *sym, const char *fromsec) +{ + const char *tosec; + Elf_Sym *to; + Elf_Sym *from; + const char *tosym; + const char *fromsym; + + from = find_elf_symbol2(elf, r->r_offset, fromsec); + fromsym = sym_name(elf, from); + + if (strstarts(fromsym, "reference___initcall")) + return; + + tosec = sec_name(elf, get_secindex(elf, sym)); + to = find_elf_symbol(elf, r->r_addend, sym); + tosym = sym_name(elf, to); + + /* check whitelist - we may ignore it */ + if (secref_whitelist(mismatch, + fromsec, fromsym, tosec, tosym)) { + report_sec_mismatch(modname, mismatch, + fromsec, r->r_offset, fromsym, + is_function(from), tosec, tosym, + is_function(to)); + } +} + +static int is_executable_section(struct elf_info *elf, unsigned int section_index) +{ + if (section_index > elf->num_sections) + fatal("section_index is outside elf->num_sections!\n"); + + return ((elf->sechdrs[section_index].sh_flags & SHF_EXECINSTR) == SHF_EXECINSTR); +} + +/* + * We rely on a gross hack in section_rel[a]() calling find_extable_entry_size() + * to know the sizeof(struct exception_table_entry) for the target architecture. + */ +static unsigned int extable_entry_size; +static void find_extable_entry_size(const char *sec, const Elf_Rela *r) +{ + /* + * If we're currently checking the second relocation within __ex_table, + * that relocation offset tells us the offsetof(struct + * exception_table_entry, fixup) which is equal to sizeof(struct + * exception_table_entry) divided by two. We use that to our advantage + * since there's no portable way to get that size as every architecture + * seems to go with different sized types. Not pretty but better than + * hard-coding the size for every architecture.. + */ + if (!extable_entry_size) + extable_entry_size = r->r_offset * 2; +} + +static inline bool is_extable_fault_address(Elf_Rela *r) +{ + /* + * extable_entry_size is only discovered after we've handled the + * _second_ relocation in __ex_table, so only abort when we're not + * handling the first reloc and extable_entry_size is zero. + */ + if (r->r_offset && extable_entry_size == 0) + fatal("extable_entry size hasn't been discovered!\n"); + + return ((r->r_offset == 0) || + (r->r_offset % extable_entry_size == 0)); +} + +#define is_second_extable_reloc(Start, Cur, Sec) \ + (((Cur) == (Start) + 1) && (strcmp("__ex_table", (Sec)) == 0)) + +static void report_extable_warnings(const char *modname, struct elf_info *elf, + const struct sectioncheck *mismatch, + Elf_Rela *r, Elf_Sym *sym, + const char *fromsec, const char *tosec) +{ + Elf_Sym *fromsym = find_elf_symbol2(elf, r->r_offset, fromsec); + const char *fromsym_name = sym_name(elf, fromsym); + Elf_Sym *tosym = find_elf_symbol(elf, r->r_addend, sym); + const char *tosym_name = sym_name(elf, tosym); + const char *from_pretty_name; + const char *from_pretty_name_p; + const char *to_pretty_name; + const char *to_pretty_name_p; + + get_pretty_name(is_function(fromsym), + &from_pretty_name, &from_pretty_name_p); + get_pretty_name(is_function(tosym), + &to_pretty_name, &to_pretty_name_p); + + warn("%s(%s+0x%lx): Section mismatch in reference from the %s %s%s to the %s %s:%s%s\n", + modname, fromsec, (long)r->r_offset, from_pretty_name, + fromsym_name, from_pretty_name_p, + to_pretty_name, tosec, tosym_name, to_pretty_name_p); + + if (!match(tosec, mismatch->bad_tosec) && + is_executable_section(elf, get_secindex(elf, sym))) + fprintf(stderr, + "The relocation at %s+0x%lx references\n" + "section \"%s\" which is not in the list of\n" + "authorized sections. If you're adding a new section\n" + "and/or if this reference is valid, add \"%s\" to the\n" + "list of authorized sections to jump to on fault.\n" + "This can be achieved by adding \"%s\" to\n" + "OTHER_TEXT_SECTIONS in scripts/mod/modpost.c.\n", + fromsec, (long)r->r_offset, tosec, tosec, tosec); +} + +static void extable_mismatch_handler(const char *modname, struct elf_info *elf, + const struct sectioncheck *mismatch, + Elf_Rela *r, Elf_Sym *sym, const char *fromsec) +{ + const char *tosec = sec_name(elf, get_secindex(elf, sym)); + + sec_mismatch_count++; + + report_extable_warnings(modname, elf, mismatch, r, sym, fromsec, tosec); + + if (match(tosec, mismatch->bad_tosec)) + fatal("The relocation at %s+0x%lx references\n" + "section \"%s\" which is black-listed.\n" + "Something is seriously wrong and should be fixed.\n" + "You might get more information about where this is\n" + "coming from by using scripts/check_extable.sh %s\n", + fromsec, (long)r->r_offset, tosec, modname); + else if (!is_executable_section(elf, get_secindex(elf, sym))) { + if (is_extable_fault_address(r)) + fatal("The relocation at %s+0x%lx references\n" + "section \"%s\" which is not executable, IOW\n" + "it is not possible for the kernel to fault\n" + "at that address. Something is seriously wrong\n" + "and should be fixed.\n", + fromsec, (long)r->r_offset, tosec); + else + fatal("The relocation at %s+0x%lx references\n" + "section \"%s\" which is not executable, IOW\n" + "the kernel will fault if it ever tries to\n" + "jump to it. Something is seriously wrong\n" + "and should be fixed.\n", + fromsec, (long)r->r_offset, tosec); + } +} + +static void check_section_mismatch(const char *modname, struct elf_info *elf, + Elf_Rela *r, Elf_Sym *sym, const char *fromsec) +{ + const char *tosec = sec_name(elf, get_secindex(elf, sym)); + const struct sectioncheck *mismatch = section_mismatch(fromsec, tosec); + + if (mismatch) { + if (mismatch->handler) + mismatch->handler(modname, elf, mismatch, + r, sym, fromsec); + else + default_mismatch_handler(modname, elf, mismatch, + r, sym, fromsec); + } +} + +static unsigned int *reloc_location(struct elf_info *elf, + Elf_Shdr *sechdr, Elf_Rela *r) +{ + return sym_get_data_by_offset(elf, sechdr->sh_info, r->r_offset); +} + +static int addend_386_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r) +{ + unsigned int r_typ = ELF_R_TYPE(r->r_info); + unsigned int *location = reloc_location(elf, sechdr, r); + + switch (r_typ) { + case R_386_32: + r->r_addend = TO_NATIVE(*location); + break; + case R_386_PC32: + r->r_addend = TO_NATIVE(*location) + 4; + /* For CONFIG_RELOCATABLE=y */ + if (elf->hdr->e_type == ET_EXEC) + r->r_addend += r->r_offset; + break; + } + return 0; +} + +#ifndef R_ARM_CALL +#define R_ARM_CALL 28 +#endif +#ifndef R_ARM_JUMP24 +#define R_ARM_JUMP24 29 +#endif + +#ifndef R_ARM_THM_CALL +#define R_ARM_THM_CALL 10 +#endif +#ifndef R_ARM_THM_JUMP24 +#define R_ARM_THM_JUMP24 30 +#endif +#ifndef R_ARM_THM_JUMP19 +#define R_ARM_THM_JUMP19 51 +#endif + +static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r) +{ + unsigned int r_typ = ELF_R_TYPE(r->r_info); + + switch (r_typ) { + case R_ARM_ABS32: + /* From ARM ABI: (S + A) | T */ + r->r_addend = (int)(long) + (elf->symtab_start + ELF_R_SYM(r->r_info)); + break; + case R_ARM_PC24: + case R_ARM_CALL: + case R_ARM_JUMP24: + case R_ARM_THM_CALL: + case R_ARM_THM_JUMP24: + case R_ARM_THM_JUMP19: + /* From ARM ABI: ((S + A) | T) - P */ + r->r_addend = (int)(long)(elf->hdr + + sechdr->sh_offset + + (r->r_offset - sechdr->sh_addr)); + break; + default: + return 1; + } + return 0; +} + +static int addend_mips_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r) +{ + unsigned int r_typ = ELF_R_TYPE(r->r_info); + unsigned int *location = reloc_location(elf, sechdr, r); + unsigned int inst; + + if (r_typ == R_MIPS_HI16) + return 1; /* skip this */ + inst = TO_NATIVE(*location); + switch (r_typ) { + case R_MIPS_LO16: + r->r_addend = inst & 0xffff; + break; + case R_MIPS_26: + r->r_addend = (inst & 0x03ffffff) << 2; + break; + case R_MIPS_32: + r->r_addend = inst; + break; + } + return 0; +} + +#ifndef EM_RISCV +#define EM_RISCV 243 +#endif + +#ifndef R_RISCV_SUB32 +#define R_RISCV_SUB32 39 +#endif + +static void section_rela(const char *modname, struct elf_info *elf, + Elf_Shdr *sechdr) +{ + Elf_Sym *sym; + Elf_Rela *rela; + Elf_Rela r; + unsigned int r_sym; + const char *fromsec; + + Elf_Rela *start = (void *)elf->hdr + sechdr->sh_offset; + Elf_Rela *stop = (void *)start + sechdr->sh_size; + + fromsec = sech_name(elf, sechdr); + fromsec += strlen(".rela"); + /* if from section (name) is know good then skip it */ + if (match(fromsec, section_white_list)) + return; + + for (rela = start; rela < stop; rela++) { + r.r_offset = TO_NATIVE(rela->r_offset); +#if KERNEL_ELFCLASS == ELFCLASS64 + if (elf->hdr->e_machine == EM_MIPS) { + unsigned int r_typ; + + r_sym = ELF64_MIPS_R_SYM(rela->r_info); + r_sym = TO_NATIVE(r_sym); + r_typ = ELF64_MIPS_R_TYPE(rela->r_info); + r.r_info = ELF64_R_INFO(r_sym, r_typ); + } else { + r.r_info = TO_NATIVE(rela->r_info); + r_sym = ELF_R_SYM(r.r_info); + } +#else + r.r_info = TO_NATIVE(rela->r_info); + r_sym = ELF_R_SYM(r.r_info); +#endif + r.r_addend = TO_NATIVE(rela->r_addend); + switch (elf->hdr->e_machine) { + case EM_RISCV: + if (!strcmp("__ex_table", fromsec) && + ELF_R_TYPE(r.r_info) == R_RISCV_SUB32) + continue; + break; + } + sym = elf->symtab_start + r_sym; + /* Skip special sections */ + if (is_shndx_special(sym->st_shndx)) + continue; + if (is_second_extable_reloc(start, rela, fromsec)) + find_extable_entry_size(fromsec, &r); + check_section_mismatch(modname, elf, &r, sym, fromsec); + } +} + +static void section_rel(const char *modname, struct elf_info *elf, + Elf_Shdr *sechdr) +{ + Elf_Sym *sym; + Elf_Rel *rel; + Elf_Rela r; + unsigned int r_sym; + const char *fromsec; + + Elf_Rel *start = (void *)elf->hdr + sechdr->sh_offset; + Elf_Rel *stop = (void *)start + sechdr->sh_size; + + fromsec = sech_name(elf, sechdr); + fromsec += strlen(".rel"); + /* if from section (name) is know good then skip it */ + if (match(fromsec, section_white_list)) + return; + + for (rel = start; rel < stop; rel++) { + r.r_offset = TO_NATIVE(rel->r_offset); +#if KERNEL_ELFCLASS == ELFCLASS64 + if (elf->hdr->e_machine == EM_MIPS) { + unsigned int r_typ; + + r_sym = ELF64_MIPS_R_SYM(rel->r_info); + r_sym = TO_NATIVE(r_sym); + r_typ = ELF64_MIPS_R_TYPE(rel->r_info); + r.r_info = ELF64_R_INFO(r_sym, r_typ); + } else { + r.r_info = TO_NATIVE(rel->r_info); + r_sym = ELF_R_SYM(r.r_info); + } +#else + r.r_info = TO_NATIVE(rel->r_info); + r_sym = ELF_R_SYM(r.r_info); +#endif + r.r_addend = 0; + switch (elf->hdr->e_machine) { + case EM_386: + if (addend_386_rel(elf, sechdr, &r)) + continue; + break; + case EM_ARM: + if (addend_arm_rel(elf, sechdr, &r)) + continue; + break; + case EM_MIPS: + if (addend_mips_rel(elf, sechdr, &r)) + continue; + break; + } + sym = elf->symtab_start + r_sym; + /* Skip special sections */ + if (is_shndx_special(sym->st_shndx)) + continue; + if (is_second_extable_reloc(start, rel, fromsec)) + find_extable_entry_size(fromsec, &r); + check_section_mismatch(modname, elf, &r, sym, fromsec); + } +} + +/* + * A module includes a number of sections that are discarded + * either when loaded or when used as built-in. + * For loaded modules all functions marked __init and all data + * marked __initdata will be discarded when the module has been initialized. + * Likewise for modules used built-in the sections marked __exit + * are discarded because __exit marked function are supposed to be called + * only when a module is unloaded which never happens for built-in modules. + * The check_sec_ref() function traverses all relocation records + * to find all references to a section that reference a section that will + * be discarded and warns about it. + */ +void check_sec_ref(struct module *mod, const char *modname, + 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(modname, elf, &elf->sechdrs[i]); + /* We want to process only relocation sections and not .init */ + if (sechdrs[i].sh_type == SHT_RELA) + section_rela(modname, elf, &elf->sechdrs[i]); + else if (sechdrs[i].sh_type == SHT_REL) + section_rel(modname, elf, &elf->sechdrs[i]); + } +} -- 2.32.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v4 03/14] modpost: split the section mismatch checks into section-check.c 2022-05-08 19:06 ` [PATCH v4 03/14] modpost: split the section mismatch checks into section-check.c Masahiro Yamada @ 2022-05-09 17:19 ` Nick Desaulniers 2022-05-10 6:55 ` Masahiro Yamada 0 siblings, 1 reply; 33+ messages in thread From: Nick Desaulniers @ 2022-05-09 17:19 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, linux-kernel, Nathan Chancellor, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, Ard Biesheuvel, Sami Tolvanen, clang-built-linux On Sun, May 8, 2022 at 12:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > modpost.c is too big, and the half of the code is for section checks. > Split it. > > I fixed some style issues in the moved code. It would be helpful for review if the split and restyle were distinct patches. Otherwise I can't tell what has changed. This does lose the ability to use git blame to get more context on some of the oddities in modpost (which I have found useful in the past). I don't feel strongly though. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > Changes in v4: > - New patch > > scripts/mod/Makefile | 2 +- > scripts/mod/modpost.c | 1202 +--------------------------------- > scripts/mod/modpost.h | 34 +- > scripts/mod/section-check.c | 1222 +++++++++++++++++++++++++++++++++++ > 4 files changed, 1240 insertions(+), 1220 deletions(-) > create mode 100644 scripts/mod/section-check.c > > diff --git a/scripts/mod/Makefile b/scripts/mod/Makefile > index c9e38ad937fd..ca739c6c68a1 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 section-check.o file2alias.o sumversion.o > > devicetable-offsets-file := devicetable-offsets.h > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index a78b75f0eeb0..e7e2c70a98f5 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -31,7 +31,7 @@ static bool external_module; > /* Only warn about unresolved symbols */ > static bool warn_unresolved; > > -static int sec_mismatch_count; > +int sec_mismatch_count; ^ this should go in modpost.h if it is to be used in two translation units, rather than forward declaring it in section-check.c. You did this for the functions. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 03/14] modpost: split the section mismatch checks into section-check.c 2022-05-09 17:19 ` Nick Desaulniers @ 2022-05-10 6:55 ` Masahiro Yamada 2022-05-11 18:47 ` Nick Desaulniers 0 siblings, 1 reply; 33+ messages in thread From: Masahiro Yamada @ 2022-05-10 6:55 UTC (permalink / raw) To: Nick Desaulniers Cc: Linux Kbuild mailing list, Linux Kernel Mailing List, Nathan Chancellor, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, Ard Biesheuvel, Sami Tolvanen, clang-built-linux On Tue, May 10, 2022 at 2:20 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Sun, May 8, 2022 at 12:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > modpost.c is too big, and the half of the code is for section checks. > > Split it. > > > > I fixed some style issues in the moved code. > > It would be helpful for review if the split and restyle were distinct > patches. Otherwise I can't tell what has changed. > > This does lose the ability to use git blame to get more context on > some of the oddities in modpost (which I have found useful in the > past). I don't feel strongly though. OK, I will just move the code in v5. > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > > index a78b75f0eeb0..e7e2c70a98f5 100644 > > --- a/scripts/mod/modpost.c > > +++ b/scripts/mod/modpost.c > > @@ -31,7 +31,7 @@ static bool external_module; > > /* Only warn about unresolved symbols */ > > static bool warn_unresolved; > > > > -static int sec_mismatch_count; > > +int sec_mismatch_count; > > ^ this should go in modpost.h if it is to be used in two translation > units, rather than forward declaring it in section-check.c. You did > this for the functions. Sorry, I do not understand. In modpost.h, I put the declaration: extern int sec_mismatch_count; If I moved it to the header without 'extern' I would get multiple definitions. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 03/14] modpost: split the section mismatch checks into section-check.c 2022-05-10 6:55 ` Masahiro Yamada @ 2022-05-11 18:47 ` Nick Desaulniers 2022-05-11 19:27 ` Masahiro Yamada 0 siblings, 1 reply; 33+ messages in thread From: Nick Desaulniers @ 2022-05-11 18:47 UTC (permalink / raw) To: Masahiro Yamada Cc: Linux Kbuild mailing list, Linux Kernel Mailing List, Nathan Chancellor, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, Ard Biesheuvel, Sami Tolvanen, clang-built-linux On Mon, May 9, 2022 at 11:57 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > > > index a78b75f0eeb0..e7e2c70a98f5 100644 > > > --- a/scripts/mod/modpost.c > > > +++ b/scripts/mod/modpost.c > > > @@ -31,7 +31,7 @@ static bool external_module; > > > /* Only warn about unresolved symbols */ > > > static bool warn_unresolved; > > > > > > -static int sec_mismatch_count; > > > +int sec_mismatch_count; > > > > ^ this should go in modpost.h if it is to be used in two translation > > units, rather than forward declaring it in section-check.c. You did > > this for the functions. > > > Sorry, I do not understand. > > > In modpost.h, I put the declaration: > > extern int sec_mismatch_count; > > If I moved it to the header without 'extern' > I would get multiple definitions. Yeah, you need to _declare_ it w/ extern in the header, then _define_ it in one source file. That way, if the type ever changes, the sources will agree on type in all source files. You will get a redefinition error if the definition changes the type of the variable since the last declaration. What you're doing is forward declaring, which works, and is a common pattern for (bloated) C++, but is less type safe than sharing a single common declaration between multiple source files via a single common shared header. (Sorry I didn't respond before you sent v5) -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 03/14] modpost: split the section mismatch checks into section-check.c 2022-05-11 18:47 ` Nick Desaulniers @ 2022-05-11 19:27 ` Masahiro Yamada 2022-05-11 19:51 ` Jeff Johnson 0 siblings, 1 reply; 33+ messages in thread From: Masahiro Yamada @ 2022-05-11 19:27 UTC (permalink / raw) To: Nick Desaulniers Cc: Linux Kbuild mailing list, Linux Kernel Mailing List, Nathan Chancellor, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, Ard Biesheuvel, Sami Tolvanen, clang-built-linux On Thu, May 12, 2022 at 3:48 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Mon, May 9, 2022 at 11:57 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > > > > index a78b75f0eeb0..e7e2c70a98f5 100644 > > > > --- a/scripts/mod/modpost.c > > > > +++ b/scripts/mod/modpost.c > > > > @@ -31,7 +31,7 @@ static bool external_module; > > > > /* Only warn about unresolved symbols */ > > > > static bool warn_unresolved; > > > > > > > > -static int sec_mismatch_count; > > > > +int sec_mismatch_count; > > > > > > ^ this should go in modpost.h if it is to be used in two translation > > > units, rather than forward declaring it in section-check.c. You did > > > this for the functions. > > > > > > Sorry, I do not understand. > > > > > > In modpost.h, I put the declaration: > > > > extern int sec_mismatch_count; > > > > If I moved it to the header without 'extern' > > I would get multiple definitions. > > Yeah, you need to _declare_ it w/ extern in the header, then _define_ > it in one source file. > > That way, if the type ever changes, the sources will agree on type in > all source files. You will get a redefinition error if the definition > changes the type of the variable since the last declaration. > > What you're doing is forward declaring, which works, and is a common > pattern for (bloated) C++, but is less type safe than sharing a single > common declaration between multiple source files via a single common > shared header. (Sorry I didn't respond before you sent v5) Sorry, I still do not understand your suggestion. Could you provide me with a code diff showing how to do this better? -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 03/14] modpost: split the section mismatch checks into section-check.c 2022-05-11 19:27 ` Masahiro Yamada @ 2022-05-11 19:51 ` Jeff Johnson 2022-05-11 20:01 ` Nick Desaulniers 0 siblings, 1 reply; 33+ messages in thread From: Jeff Johnson @ 2022-05-11 19:51 UTC (permalink / raw) To: Masahiro Yamada, Nick Desaulniers Cc: Linux Kbuild mailing list, Linux Kernel Mailing List, Nathan Chancellor, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, Ard Biesheuvel, Sami Tolvanen, clang-built-linux On 5/11/2022 12:27 PM, Masahiro Yamada wrote: > On Thu, May 12, 2022 at 3:48 AM Nick Desaulniers > <ndesaulniers@google.com> wrote: >> >> On Mon, May 9, 2022 at 11:57 PM Masahiro Yamada <masahiroy@kernel.org> wrote: >>> >>>>> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c >>>>> index a78b75f0eeb0..e7e2c70a98f5 100644 >>>>> --- a/scripts/mod/modpost.c >>>>> +++ b/scripts/mod/modpost.c >>>>> @@ -31,7 +31,7 @@ static bool external_module; >>>>> /* Only warn about unresolved symbols */ >>>>> static bool warn_unresolved; >>>>> >>>>> -static int sec_mismatch_count; >>>>> +int sec_mismatch_count; >>>> >>>> ^ this should go in modpost.h if it is to be used in two translation >>>> units, rather than forward declaring it in section-check.c. You did >>>> this for the functions. >>> >>> >>> Sorry, I do not understand. >>> >>> >>> In modpost.h, I put the declaration: >>> >>> extern int sec_mismatch_count; >>> >>> If I moved it to the header without 'extern' >>> I would get multiple definitions. >> >> Yeah, you need to _declare_ it w/ extern in the header, then _define_ >> it in one source file. >> >> That way, if the type ever changes, the sources will agree on type in >> all source files. You will get a redefinition error if the definition >> changes the type of the variable since the last declaration. >> >> What you're doing is forward declaring, which works, and is a common >> pattern for (bloated) C++, but is less type safe than sharing a single >> common declaration between multiple source files via a single common >> shared header. (Sorry I didn't respond before you sent v5) > > Sorry, I still do not understand your suggestion. > > > Could you provide me with a code diff > showing how to do this better? I think you are doing exactly what he's asking for: declare it with extern in the header (modpost.h change) define it in one source file (modpost.c change) ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 03/14] modpost: split the section mismatch checks into section-check.c 2022-05-11 19:51 ` Jeff Johnson @ 2022-05-11 20:01 ` Nick Desaulniers 0 siblings, 0 replies; 33+ messages in thread From: Nick Desaulniers @ 2022-05-11 20:01 UTC (permalink / raw) To: Jeff Johnson Cc: Masahiro Yamada, Linux Kbuild mailing list, Linux Kernel Mailing List, Nathan Chancellor, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, Ard Biesheuvel, Sami Tolvanen, clang-built-linux On Wed, May 11, 2022 at 12:51 PM Jeff Johnson <quic_jjohnson@quicinc.com> wrote: > > On 5/11/2022 12:27 PM, Masahiro Yamada wrote: > > On Thu, May 12, 2022 at 3:48 AM Nick Desaulniers > > <ndesaulniers@google.com> wrote: > >> > >> On Mon, May 9, 2022 at 11:57 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > >>> > >>>>> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > >>>>> index a78b75f0eeb0..e7e2c70a98f5 100644 > >>>>> --- a/scripts/mod/modpost.c > >>>>> +++ b/scripts/mod/modpost.c > >>>>> @@ -31,7 +31,7 @@ static bool external_module; > >>>>> /* Only warn about unresolved symbols */ > >>>>> static bool warn_unresolved; > >>>>> > >>>>> -static int sec_mismatch_count; > >>>>> +int sec_mismatch_count; > >>>> > >>>> ^ this should go in modpost.h if it is to be used in two translation > >>>> units, rather than forward declaring it in section-check.c. You did > >>>> this for the functions. > >>> > >>> > >>> Sorry, I do not understand. > >>> > >>> > >>> In modpost.h, I put the declaration: > >>> > >>> extern int sec_mismatch_count; > >>> > >>> If I moved it to the header without 'extern' > >>> I would get multiple definitions. > >> > >> Yeah, you need to _declare_ it w/ extern in the header, then _define_ > >> it in one source file. > >> > >> That way, if the type ever changes, the sources will agree on type in > >> all source files. You will get a redefinition error if the definition > >> changes the type of the variable since the last declaration. > >> > >> What you're doing is forward declaring, which works, and is a common > >> pattern for (bloated) C++, but is less type safe than sharing a single > >> common declaration between multiple source files via a single common > >> shared header. (Sorry I didn't respond before you sent v5) > > > > Sorry, I still do not understand your suggestion. > > > > > > Could you provide me with a code diff > > showing how to do this better? > > I think you are doing exactly what he's asking for: > declare it with extern in the header (modpost.h change) > define it in one source file (modpost.c change) > Oh! Indeed, sorry, I'm getting lost here in the review. Sorry for the noise then. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v4 04/14] modpost: add sym_find_with_module() helper 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada ` (2 preceding siblings ...) 2022-05-08 19:06 ` [PATCH v4 03/14] modpost: split the section mismatch checks into section-check.c Masahiro Yamada @ 2022-05-08 19:06 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 05/14] modpost: extract symbol versions from *.cmd files Masahiro Yamada ` (10 subsequent siblings) 14 siblings, 0 replies; 33+ messages in thread From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada find_symbol() returns the first symbol found in the hash table. This table is global, so it may return a symbol from an unexpected module. There is a case where we want to search for a symbol with a given name in a specified module. Add sym_find_with_module(), which receives the module pointer as the second argument. It is equivalent to find_module() if NULL is passed as the module pointer. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> Tested-by: Nathan Chancellor <nathan@kernel.org> --- Changes in v4: - Only takes the new helper from https://patchwork.kernel.org/project/linux-kbuild/patch/20220505072244.1155033-2-masahiroy@kernel.org/ Changes in v2: - Rename the new func to sym_find_with_module() scripts/mod/modpost.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index e7e2c70a98f5..fc5db1f73cf1 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -266,7 +266,7 @@ static void sym_add_unresolved(const char *name, struct module *mod, bool weak) list_add_tail(&sym->list, &mod->unresolved_symbols); } -static struct symbol *find_symbol(const char *name) +static struct symbol *sym_find_with_module(const char *name, struct module *mod) { struct symbol *s; @@ -275,12 +275,17 @@ static struct symbol *find_symbol(const char *name) name++; for (s = symbolhash[tdb_hash(name) % SYMBOL_HASH_SIZE]; s; s = s->next) { - if (strcmp(s->name, name) == 0) + if (strcmp(s->name, name) == 0 && (!mod || s->module == mod)) return s; } return NULL; } +static struct symbol *find_symbol(const char *name) +{ + return sym_find_with_module(name, NULL); +} + struct namespace_list { struct list_head list; char namespace[]; -- 2.32.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v4 05/14] modpost: extract symbol versions from *.cmd files 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada ` (3 preceding siblings ...) 2022-05-08 19:06 ` [PATCH v4 04/14] modpost: add sym_find_with_module() helper Masahiro Yamada @ 2022-05-08 19:06 ` Masahiro Yamada 2022-05-09 21:52 ` Sami Tolvanen 2022-05-08 19:06 ` [PATCH v4 06/14] kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS Masahiro Yamada ` (9 subsequent siblings) 14 siblings, 1 reply; 33+ messages in thread From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada Currently, CONFIG_MODVERSIONS needs extra link to embed the symbol versions into ELF objects. Then, modpost extracts the version CRCs from them. The following figures show how it currently works, and how I am trying to change it. Current implementation ====================== |----------| embed CRC -------------------------->| final | $(CC) $(LD) / |---------| | link for | -----> *.o -------> *.o -->| modpost | | vmlinux | / / | |-- *.mod.c -->| or | / genksyms / |---------| | module | *.c ------> *.symversions |----------| Genksyms outputs the calculated CRCs in the form of linker script (*.symversions), which is used by $(LD) to update the object. If CONFIG_LTO_CLANG=y, the build process is much more complex. Embedding the CRCs is postponed until the LLVM bitcode is converted into ELF, creating another intermediate *.prelink.o. However, this complexity is unneeded. There is no reason why we must embed version CRCs in objects so early. There is final link stage for vmlinux (scripts/link-vmlinux.sh) and modules (scripts/Makefile.modfinal). We can link CRCs at the very last moment. New implementation ================== |----------| --------------------------------------->| final | $(CC) / |---------| | link for | -----> *.o ---->| | | vmlinux | / | modpost |--- .vmlinux.export.c -->| or | / genksyms | |--- *.mod.c ------------>| module | *.c ------> *.cmd -->|---------| |----------| Pass the symbol versions to modpost as separate text data, which are available in *.cmd files. This commit changes modpost to extract CRCs from *.cmd files instead of from ELF objects. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> Tested-by: Nathan Chancellor <nathan@kernel.org> --- (no changes since v2) Changes in v2: - Simplify the implementation (parse .cmd files after ELF) scripts/mod/modpost.c | 177 ++++++++++++++++++++++++++++++------------ 1 file changed, 129 insertions(+), 48 deletions(-) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index fc5db1f73cf1..54f957952723 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -381,19 +381,10 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod, return s; } -static void sym_set_crc(const char *name, unsigned int crc) +static void sym_set_crc(struct symbol *sym, unsigned int crc) { - struct symbol *s = find_symbol(name); - - /* - * Ignore stand-alone __crc_*, which might be auto-generated symbols - * such as __*_veneer in ARM ELF. - */ - if (!s) - return; - - s->crc = crc; - s->crc_valid = true; + sym->crc = crc; + sym->crc_valid = true; } static void *grab_file(const char *filename, size_t *size) @@ -616,33 +607,6 @@ static int ignore_undef_symbol(struct elf_info *info, const char *symname) return 0; } -static void handle_modversion(const struct module *mod, - const struct elf_info *info, - const Elf_Sym *sym, const char *symname) -{ - unsigned int crc; - - if (sym->st_shndx == SHN_UNDEF) { - warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n" - "Is \"%s\" prototyped in <asm/asm-prototypes.h>?\n", - symname, mod->name, mod->is_vmlinux ? "" : ".ko", - symname); - - return; - } - - if (sym->st_shndx == SHN_ABS) { - crc = sym->st_value; - } else { - unsigned int *crcp; - - /* symbol points to the CRC in the ELF object */ - crcp = sym_get_data(info, sym); - crc = TO_NATIVE(*crcp); - } - sym_set_crc(symname, crc); -} - static void handle_symbol(struct module *mod, struct elf_info *info, const Elf_Sym *sym, const char *symname) { @@ -760,6 +724,102 @@ static char *remove_dot(char *s) return s; } +/* + * The CRCs are recorded in .*.cmd files in the form of: + * #SYMVER <name> <crc> + */ +static void extract_crcs_for_object(const char *object, struct module *mod) +{ + char cmd_file[PATH_MAX]; + char *buf, *p; + const char *base; + int dirlen, ret; + + base = strrchr(object, '/'); + if (base) { + base++; + dirlen = base - object; + } else { + dirlen = 0; + base = object; + } + + ret = snprintf(cmd_file, sizeof(cmd_file), "%.*s.%s.cmd", + dirlen, object, base); + if (ret >= sizeof(cmd_file)) { + error("%s: too long path was truncated\n", cmd_file); + return; + } + + buf = read_text_file(cmd_file); + p = buf; + + while ((p = strstr(p, "\n#SYMVER "))) { + char *name; + size_t namelen; + unsigned int crc; + struct symbol *sym; + + name = p + strlen("\n#SYMVER "); + + p = strchr(name, ' '); + if (!p) + break; + + namelen = p - name; + p++; + + if (!isdigit(*p)) + continue; /* skip this line */ + + crc = strtol(p, &p, 0); + if (*p != '\n') + continue; /* skip this line */ + + name[namelen] = '\0'; + + sym = sym_find_with_module(name, mod); + if (!sym) { + warn("Skip the version for unexported symbol \"%s\" [%s%s]\n", + name, mod->name, mod->is_vmlinux ? "" : ".ko"); + continue; + } + sym_set_crc(sym, crc); + } + + free(buf); +} + +/* + * The symbol versions (CRC) are recorded in the .*.cmd files. + * Parse them to retrieve CRCs for the current module. + */ +static void mod_set_crcs(struct module *mod) +{ + char objlist[PATH_MAX]; + char *buf, *p, *obj; + int ret; + + if (mod->is_vmlinux) { + strcpy(objlist, ".vmlinux.objs"); + } else { + /* objects for a module are listed in the *.mod file. */ + ret = snprintf(objlist, sizeof(objlist), "%s.mod", mod->name); + if (ret >= sizeof(objlist)) { + error("%s: too long path was truncated\n", objlist); + return; + } + } + + buf = read_text_file(objlist); + p = buf; + + while ((obj = strsep(&p, "\n")) && obj[0]) + extract_crcs_for_object(obj, mod); + + free(buf); +} + static void read_symbols(const char *modname) { const char *symname; @@ -820,9 +880,6 @@ static void read_symbols(const char *modname) if (strstarts(symname, "__kstrtabns_")) sym_update_namespace(symname + strlen("__kstrtabns_"), sym_get_data(&info, sym)); - if (strstarts(symname, "__crc_")) - handle_modversion(mod, &info, sym, - symname + strlen("__crc_")); } // check for static EXPORT_SYMBOL_* functions && global vars @@ -850,12 +907,17 @@ static void read_symbols(const char *modname) parse_elf_finish(&info); - /* Our trick to get versioning for module struct etc. - it's - * never passed as an argument to an exported function, so - * the automatic versioning doesn't pick it up, but it's really - * important anyhow */ - if (modversions) + if (modversions) { + /* + * Our trick to get versioning for module struct etc. - it's + * never passed as an argument to an exported function, so + * the automatic versioning doesn't pick it up, but it's really + * important anyhow + */ sym_add_unresolved("module_layout", mod, false); + + mod_set_crcs(mod); + } } static void read_symbols_from_files(const char *filename) @@ -1012,6 +1074,23 @@ static void add_header(struct buffer *b, struct module *mod) buf_printf(b, "\nMODULE_INFO(staging, \"Y\");\n"); } +static void check_symversions(struct module *mod) +{ + struct symbol *sym; + + if (!modversions) + return; + + list_for_each_entry(sym, &mod->exported_symbols, list) { + if (!sym->crc_valid) { + warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n" + "Is \"%s\" prototyped in <asm/asm-prototypes.h>?\n", + sym->name, mod->name, mod->is_vmlinux ? "" : ".ko", + sym->name); + } + } +} + /** * Record CRCs for unresolved symbols **/ @@ -1227,7 +1306,7 @@ static void read_dump(const char *fname) } s = sym_add_exported(symname, mod, gpl_only); s->is_static = false; - sym_set_crc(symname, crc); + sym_set_crc(s, crc); sym_update_namespace(symname, namespace); } free(buf); @@ -1353,6 +1432,8 @@ int main(int argc, char **argv) if (mod->from_dump) continue; + check_symversions(mod); + if (!mod->is_vmlinux) write_mod_c_file(mod); } -- 2.32.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v4 05/14] modpost: extract symbol versions from *.cmd files 2022-05-08 19:06 ` [PATCH v4 05/14] modpost: extract symbol versions from *.cmd files Masahiro Yamada @ 2022-05-09 21:52 ` Sami Tolvanen 0 siblings, 0 replies; 33+ messages in thread From: Sami Tolvanen @ 2022-05-09 21:52 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, LKML, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel On Sun, May 8, 2022 at 12:09 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Currently, CONFIG_MODVERSIONS needs extra link to embed the symbol > versions into ELF objects. Then, modpost extracts the version CRCs > from them. > > The following figures show how it currently works, and how I am trying > to change it. > > Current implementation > ====================== > |----------| > embed CRC -------------------------->| final | > $(CC) $(LD) / |---------| | link for | > -----> *.o -------> *.o -->| modpost | | vmlinux | > / / | |-- *.mod.c -->| or | > / genksyms / |---------| | module | > *.c ------> *.symversions |----------| > > Genksyms outputs the calculated CRCs in the form of linker script > (*.symversions), which is used by $(LD) to update the object. > > If CONFIG_LTO_CLANG=y, the build process is much more complex. Embedding > the CRCs is postponed until the LLVM bitcode is converted into ELF, > creating another intermediate *.prelink.o. > > However, this complexity is unneeded. There is no reason why we must > embed version CRCs in objects so early. > > There is final link stage for vmlinux (scripts/link-vmlinux.sh) and > modules (scripts/Makefile.modfinal). We can link CRCs at the very last > moment. > > New implementation > ================== > |----------| > --------------------------------------->| final | > $(CC) / |---------| | link for | > -----> *.o ---->| | | vmlinux | > / | modpost |--- .vmlinux.export.c -->| or | > / genksyms | |--- *.mod.c ------------>| module | > *.c ------> *.cmd -->|---------| |----------| > > Pass the symbol versions to modpost as separate text data, which are > available in *.cmd files. > > This commit changes modpost to extract CRCs from *.cmd files instead of > from ELF objects. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> > Tested-by: Nathan Chancellor <nathan@kernel.org> This looks good to me, it's great to get rid of the .symversions files! Reviewed-by: Sami Tolvanen <samitolvanen@google.com> Sami ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v4 06/14] kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada ` (4 preceding siblings ...) 2022-05-08 19:06 ` [PATCH v4 05/14] modpost: extract symbol versions from *.cmd files Masahiro Yamada @ 2022-05-08 19:06 ` Masahiro Yamada 2022-05-09 17:50 ` Nick Desaulniers 2022-05-08 19:06 ` [PATCH v4 07/14] kbuild: stop merging *.symversions Masahiro Yamada ` (8 subsequent siblings) 14 siblings, 1 reply; 33+ messages in thread From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada include/{linux,asm-generic}/export.h defines a weak symbol, __crc_* as a placeholder. Genksyms writes the version CRCs into the linker script, which will be used for filling the __crc_* symbols. The linker script format depends on CONFIG_MODULE_REL_CRCS. If it is enabled, __crc_* holds the offset to the reference of CRC. It is time to get rid of this complexity. Now that modpost parses text files (.*.cmd) to collect all the CRCs, it can generate C code that will be linked to the vmlinux or modules. Generate a new C file, .vmlinux.export.c, which contains the CRCs of symbols exported by vmlinux. It is compiled and linked to vmlinux in scripts/link-vmlinux.sh. Put the CRCs of symbols exported by modules into the existing *.mod.c files. No additional build step is needed for modules. As before, *.mod.c are compiled and linked to *.ko in scripts/Makefile.modfinal. No linker magic is used here. The new C implementation works in the same way, whether CONFIG_RELOCATABLE is enabled or not. CONFIG_MODULE_REL_CRCS is no longer needed. Previously, Kbuild invoked additional $(LD) to update the CRCs in objects, but this step is unneeded too. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Tested-by: Nathan Chancellor <nathan@kernel.org> --- Changes in v4: - Rename .vmlinux-symver.c to .vmlinux.export.c because I notice this approach is useful for further cleanups, not only for modversioning but also for overall EXPORT_SYMBOL. Changes in v3: - New patch arch/powerpc/Kconfig | 1 - arch/s390/Kconfig | 1 - arch/um/Kconfig | 1 - include/asm-generic/export.h | 22 ++++++++-------------- include/linux/export-internal.h | 16 ++++++++++++++++ include/linux/export.h | 30 ++++++++---------------------- init/Kconfig | 4 ---- kernel/module.c | 10 +--------- scripts/Makefile.build | 27 ++++----------------------- scripts/genksyms/genksyms.c | 17 ++++------------- scripts/link-vmlinux.sh | 18 +++++++++++++----- scripts/mod/modpost.c | 28 ++++++++++++++++++++++++---- 12 files changed, 78 insertions(+), 97 deletions(-) create mode 100644 include/linux/export-internal.h diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 174edabb74fa..a4e8dd889e29 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -566,7 +566,6 @@ config RELOCATABLE bool "Build a relocatable kernel" depends on PPC64 || (FLATMEM && (44x || FSL_BOOKE)) select NONSTATIC_KERNEL - select MODULE_REL_CRCS if MODVERSIONS help This builds a kernel image that is capable of running at the location the kernel is loaded at. For ppc32, there is no any diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index 77b5a03de13a..aa5848004c76 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -567,7 +567,6 @@ endchoice config RELOCATABLE bool "Build a relocatable kernel" - select MODULE_REL_CRCS if MODVERSIONS default y help This builds a kernel image that retains relocation information diff --git a/arch/um/Kconfig b/arch/um/Kconfig index 4d398b80aea8..e8983d098e73 100644 --- a/arch/um/Kconfig +++ b/arch/um/Kconfig @@ -106,7 +106,6 @@ config LD_SCRIPT_DYN bool default y depends on !LD_SCRIPT_STATIC - select MODULE_REL_CRCS if MODVERSIONS config LD_SCRIPT_DYN_RPATH bool "set rpath in the binary" if EXPERT diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h index 07a36a874dca..51ce72ce80fa 100644 --- a/include/asm-generic/export.h +++ b/include/asm-generic/export.h @@ -2,6 +2,14 @@ #ifndef __ASM_GENERIC_EXPORT_H #define __ASM_GENERIC_EXPORT_H +/* + * This comment block is used by fixdep. Please do not remove. + * + * When CONFIG_MODVERSIONS is changed from n to y, all source files having + * EXPORT_SYMBOL variants must be re-compiled because genksyms is run as a + * side effect of the .o build rule. + */ + #ifndef KSYM_FUNC #define KSYM_FUNC(x) x #endif @@ -12,9 +20,6 @@ #else #define KSYM_ALIGN 4 #endif -#ifndef KCRC_ALIGN -#define KCRC_ALIGN 4 -#endif .macro __put, val, name #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS @@ -43,17 +48,6 @@ __ksymtab_\name: __kstrtab_\name: .asciz "\name" .previous -#ifdef CONFIG_MODVERSIONS - .section ___kcrctab\sec+\name,"a" - .balign KCRC_ALIGN -#if defined(CONFIG_MODULE_REL_CRCS) - .long __crc_\name - . -#else - .long __crc_\name -#endif - .weak __crc_\name - .previous -#endif #endif .endm diff --git a/include/linux/export-internal.h b/include/linux/export-internal.h new file mode 100644 index 000000000000..77175d561058 --- /dev/null +++ b/include/linux/export-internal.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Please do not include this explicitly. + * This is used by C files generated by modpost. + */ + +#ifndef __LINUX_EXPORT_INTERNAL_H__ +#define __LINUX_EXPORT_INTERNAL_H__ + +#include <linux/compiler.h> +#include <linux/types.h> + +#define SYMBOL_CRC(sym, crc, sec) \ + u32 __section("___kcrctab" sec "+" #sym) __crc_##sym = crc + +#endif /* __LINUX_EXPORT_INTERNAL_H__ */ diff --git a/include/linux/export.h b/include/linux/export.h index 27d848712b90..6c8e24e723bd 100644 --- a/include/linux/export.h +++ b/include/linux/export.h @@ -11,6 +11,14 @@ * hackers place grumpy comments in header files. */ +/* + * This comment block is used by fixdep. Please do not remove. + * + * When CONFIG_MODVERSIONS is changed from n to y, all source files having + * EXPORT_SYMBOL variants must be re-compiled because genksyms is run as a + * side effect of the .o build rule. + */ + #ifndef __ASSEMBLY__ #ifdef MODULE extern struct module __this_module; @@ -19,26 +27,6 @@ extern struct module __this_module; #define THIS_MODULE ((struct module *)0) #endif -#ifdef CONFIG_MODVERSIONS -/* Mark the CRC weak since genksyms apparently decides not to - * generate a checksums for some symbols */ -#if defined(CONFIG_MODULE_REL_CRCS) -#define __CRC_SYMBOL(sym, sec) \ - asm(" .section \"___kcrctab" sec "+" #sym "\", \"a\" \n" \ - " .weak __crc_" #sym " \n" \ - " .long __crc_" #sym " - . \n" \ - " .previous \n") -#else -#define __CRC_SYMBOL(sym, sec) \ - asm(" .section \"___kcrctab" sec "+" #sym "\", \"a\" \n" \ - " .weak __crc_" #sym " \n" \ - " .long __crc_" #sym " \n" \ - " .previous \n") -#endif -#else -#define __CRC_SYMBOL(sym, sec) -#endif - #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS #include <linux/compiler.h> /* @@ -85,7 +73,6 @@ struct kernel_symbol { /* * For every exported symbol, do the following: * - * - If applicable, place a CRC entry in the __kcrctab section. * - Put the name of the symbol and namespace (empty string "" for none) in * __ksymtab_strings. * - Place a struct kernel_symbol entry in the __ksymtab section. @@ -98,7 +85,6 @@ struct kernel_symbol { extern typeof(sym) sym; \ extern const char __kstrtab_##sym[]; \ extern const char __kstrtabns_##sym[]; \ - __CRC_SYMBOL(sym, sec); \ asm(" .section \"__ksymtab_strings\",\"aMS\",%progbits,1 \n" \ "__kstrtab_" #sym ": \n" \ " .asciz \"" #sym "\" \n" \ diff --git a/init/Kconfig b/init/Kconfig index ddcbefe535e9..f5b14318dfcb 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -2136,10 +2136,6 @@ config ASM_MODVERSIONS assembly. This can be enabled only when the target architecture supports it. -config MODULE_REL_CRCS - bool - depends on MODVERSIONS - config MODULE_SRCVERSION_ALL bool "Source checksum for all modules" help diff --git a/kernel/module.c b/kernel/module.c index 6cea788fd965..c9e2342da28e 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1231,11 +1231,6 @@ static int try_to_force_load(struct module *mod, const char *reason) #ifdef CONFIG_MODVERSIONS -static u32 resolve_rel_crc(const s32 *crc) -{ - return *(u32 *)((void *)crc + *crc); -} - static int check_version(const struct load_info *info, const char *symname, struct module *mod, @@ -1264,10 +1259,7 @@ static int check_version(const struct load_info *info, if (strcmp(versions[i].name, symname) != 0) continue; - if (IS_ENABLED(CONFIG_MODULE_REL_CRCS)) - crcval = resolve_rel_crc(crc); - else - crcval = *crc; + crcval = *crc; if (versions[i].crc == crcval) return 1; pr_debug("Found checksum %X vs module %lX\n", diff --git a/scripts/Makefile.build b/scripts/Makefile.build index a1023868775f..ddd9080fc028 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -128,7 +128,6 @@ $(obj)/%.i: $(src)/%.c FORCE genksyms = scripts/genksyms/genksyms \ $(if $(1), -T $(2)) \ - $(if $(CONFIG_MODULE_REL_CRCS), -R) \ $(if $(KBUILD_PRESERVE), -p) \ -r $(or $(wildcard $(2:.symtypes=.symref)), /dev/null) @@ -162,19 +161,11 @@ ifdef CONFIG_MODVERSIONS # o if <file>.o doesn't contain a __ksymtab version, i.e. does # not export symbols, it's done. # o otherwise, we calculate symbol versions using the good old -# genksyms on the preprocessed source and postprocess them in a way -# that they are usable as a linker script -# o generate .tmp_<file>.o from <file>.o using the linker to -# replace the unresolved symbols __crc_exported_symbol with -# the actual value of the checksum generated by genksyms -# o remove .tmp_<file>.o to <file>.o +# genksyms on the preprocessed source and dump them into the .cmd file. +# o modpost will extract versions from that file and create *.c files that will +# be compiled and linked to the kernel and/or modules. -# Generate .o.symversions files for each .o with exported symbols, and link these -# to the kernel and/or modules at the end. - -genksyms_format_rel_crc := [^_]*__crc_\([^ ]*\) = \.; LONG(\([^)]*\)).* -genksyms_format_normal := __crc_\(.*\) = \(.*\); -genksyms_format := $(if $(CONFIG_MODULE_REL_CRCS),$(genksyms_format_rel_crc),$(genksyms_format_normal)) +genksyms_format := __crc_\(.*\) = \(.*\); gen_symversions = \ if $(NM) $@ 2>/dev/null | grep -q __ksymtab; then \ @@ -188,12 +179,6 @@ gen_symversions = \ cmd_gen_symversions_c = $(call gen_symversions,c) -cmd_modversions = \ - if [ -r $@.symversions ]; then \ - $(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@ \ - -T $@.symversions; \ - mv -f $(@D)/.tmp_$(@F) $@; \ - fi endif ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT @@ -273,7 +258,6 @@ define rule_cc_o_c $(call cmd,checkdoc) $(call cmd,gen_objtooldep) $(call cmd,gen_symversions_c) - $(if $(CONFIG_LTO_CLANG),,$(call cmd,modversions)) $(call cmd,record_mcount) endef @@ -282,7 +266,6 @@ define rule_as_o_S $(call cmd,gen_ksymdeps) $(call cmd,gen_objtooldep) $(call cmd,gen_symversions_S) - $(call cmd,modversions) endef # Built-in and composite module parts @@ -296,8 +279,6 @@ ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),) quiet_cmd_cc_prelink_modules = LD [M] $@ cmd_cc_prelink_modules = \ $(LD) $(ld_flags) -r -o $@ \ - $(shell [ -s $(@:.prelink.o=.o.symversions) ] && \ - echo -T $(@:.prelink.o=.o.symversions)) \ --whole-archive $(filter-out FORCE,$^) \ $(cmd_objtool) diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c index 4827c5abe5b7..6e6933ae7911 100644 --- a/scripts/genksyms/genksyms.c +++ b/scripts/genksyms/genksyms.c @@ -33,7 +33,7 @@ char *cur_filename; int in_source_file; static int flag_debug, flag_dump_defs, flag_reference, flag_dump_types, - flag_preserve, flag_warnings, flag_rel_crcs; + flag_preserve, flag_warnings; static int errors; static int nsyms; @@ -681,10 +681,7 @@ void export_symbol(const char *name) fputs(">\n", debugfile); /* Used as a linker script. */ - printf(!flag_rel_crcs ? "__crc_%s = 0x%08lx;\n" : - "SECTIONS { .rodata : ALIGN(4) { " - "__crc_%s = .; LONG(0x%08lx); } }\n", - name, crc); + printf("__crc_%s = 0x%08lx;\n", name, crc); } } @@ -733,7 +730,6 @@ static void genksyms_usage(void) " -q, --quiet Disable warnings (default)\n" " -h, --help Print this message\n" " -V, --version Print the release version\n" - " -R, --relative-crc Emit section relative symbol CRCs\n" #else /* __GNU_LIBRARY__ */ " -s Select symbol prefix\n" " -d Increment the debug level (repeatable)\n" @@ -745,7 +741,6 @@ static void genksyms_usage(void) " -q Disable warnings (default)\n" " -h Print this message\n" " -V Print the release version\n" - " -R Emit section relative symbol CRCs\n" #endif /* __GNU_LIBRARY__ */ , stderr); } @@ -766,14 +761,13 @@ int main(int argc, char **argv) {"preserve", 0, 0, 'p'}, {"version", 0, 0, 'V'}, {"help", 0, 0, 'h'}, - {"relative-crc", 0, 0, 'R'}, {0, 0, 0, 0} }; - while ((o = getopt_long(argc, argv, "s:dwqVDr:T:phR", + while ((o = getopt_long(argc, argv, "s:dwqVDr:T:ph", &long_opts[0], NULL)) != EOF) #else /* __GNU_LIBRARY__ */ - while ((o = getopt(argc, argv, "s:dwqVDr:T:phR")) != EOF) + while ((o = getopt(argc, argv, "s:dwqVDr:T:ph")) != EOF) #endif /* __GNU_LIBRARY__ */ switch (o) { case 'd': @@ -813,9 +807,6 @@ int main(int argc, char **argv) case 'h': genksyms_usage(); return 0; - case 'R': - flag_rel_crcs = 1; - break; default: genksyms_usage(); return 1; diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index eceb3ee7ec06..6aee2401f3ad 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -88,11 +88,6 @@ modpost_link() gen_initcalls lds="-T .tmp_initcalls.lds" - if is_enabled CONFIG_MODVERSIONS; then - gen_symversions - lds="${lds} -T .tmp_symversions.lds" - fi - # This might take a while, so indicate that we're doing # an LTO link info LTO ${1} @@ -183,6 +178,10 @@ vmlinux_link() libs="${KBUILD_VMLINUX_LIBS}" fi + if is_enabled CONFIG_MODULES; then + objs="${objs} .vmlinux.export.o" + fi + if [ "${SRCARCH}" = "um" ]; then wl=-Wl, ld="${CC}" @@ -312,6 +311,7 @@ cleanup() rm -f vmlinux.o rm -f .vmlinux.d rm -f .vmlinux.objs + rm -f .vmlinux.export.c } # Use "make V=1" to debug this script @@ -373,6 +373,14 @@ if is_enabled CONFIG_DEBUG_INFO_BTF; then fi fi +if is_enabled CONFIG_MODULES; then + info CC .vmlinux.export.c + ${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} \ + ${KBUILD_CPPFLAGS} ${KBUILD_CFLAGS} \ + ${KBUILD_CFLAGS_KERNEL} ${CFLAGS_KERNEL} \ + -c -o .vmlinux.export.o .vmlinux.export.c +fi + kallsymso="" kallsymso_prev="" kallsyms_vmlinux="" diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 54f957952723..b46653d46e3d 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1040,6 +1040,7 @@ static void add_header(struct buffer *b, struct module *mod) buf_printf(b, "#define INCLUDE_VERMAGIC\n"); buf_printf(b, "#include <linux/build-salt.h>\n"); buf_printf(b, "#include <linux/elfnote-lto.h>\n"); + buf_printf(b, "#include <linux/export-internal.h>\n"); buf_printf(b, "#include <linux/vermagic.h>\n"); buf_printf(b, "#include <linux/compiler.h>\n"); buf_printf(b, "\n"); @@ -1074,20 +1075,26 @@ static void add_header(struct buffer *b, struct module *mod) buf_printf(b, "\nMODULE_INFO(staging, \"Y\");\n"); } -static void check_symversions(struct module *mod) +static void add_exported_symbols(struct buffer *buf, struct module *mod) { struct symbol *sym; if (!modversions) return; + /* record CRCs for exported symbols */ + buf_printf(buf, "\n"); list_for_each_entry(sym, &mod->exported_symbols, list) { if (!sym->crc_valid) { warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n" "Is \"%s\" prototyped in <asm/asm-prototypes.h>?\n", sym->name, mod->name, mod->is_vmlinux ? "" : ".ko", sym->name); + continue; } + + buf_printf(buf, "SYMBOL_CRC(%s, 0x%08x, \"%s\");\n", + sym->name, sym->crc, sym->is_gpl_only ? "_gpl" : ""); } } @@ -1224,6 +1231,18 @@ static void write_if_changed(struct buffer *b, const char *fname) write_buf(b, fname); } +static void write_vmlinux_export_c_file(struct module *mod) +{ + struct buffer buf = { }; + + buf_printf(&buf, + "#include <linux/export-internal.h>\n"); + + add_exported_symbols(&buf, mod); + write_if_changed(&buf, ".vmlinux.export.c"); + free(buf.p); +} + /* do sanity checks, and generate *.mod.c file */ static void write_mod_c_file(struct module *mod) { @@ -1235,6 +1254,7 @@ static void write_mod_c_file(struct module *mod) check_exports(mod); add_header(&buf, mod); + add_exported_symbols(&buf, mod); add_versions(&buf, mod); add_depends(&buf, mod); add_moddevtable(&buf, mod); @@ -1432,9 +1452,9 @@ int main(int argc, char **argv) if (mod->from_dump) continue; - check_symversions(mod); - - if (!mod->is_vmlinux) + if (mod->is_vmlinux) + write_vmlinux_export_c_file(mod); + else write_mod_c_file(mod); } -- 2.32.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v4 06/14] kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS 2022-05-08 19:06 ` [PATCH v4 06/14] kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS Masahiro Yamada @ 2022-05-09 17:50 ` Nick Desaulniers 2022-05-10 13:03 ` Masahiro Yamada 0 siblings, 1 reply; 33+ messages in thread From: Nick Desaulniers @ 2022-05-09 17:50 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, linux-kernel, Nathan Chancellor, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, Ard Biesheuvel, Sami Tolvanen, clang-built-linux On Sun, May 8, 2022 at 12:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h > index 07a36a874dca..51ce72ce80fa 100644 > --- a/include/asm-generic/export.h > +++ b/include/asm-generic/export.h > @@ -2,6 +2,14 @@ > #ifndef __ASM_GENERIC_EXPORT_H > #define __ASM_GENERIC_EXPORT_H > > +/* > + * This comment block is used by fixdep. Please do not remove. I don't know much about fixdep. How does that work, if you could summarize? > + * > + * When CONFIG_MODVERSIONS is changed from n to y, all source files having > + * EXPORT_SYMBOL variants must be re-compiled because genksyms is run as a > + * side effect of the .o build rule. > + */ > + > #ifndef KSYM_FUNC > #define KSYM_FUNC(x) x > #endif > @@ -12,9 +20,6 @@ > #else > #define KSYM_ALIGN 4 > #endif > -#ifndef KCRC_ALIGN > -#define KCRC_ALIGN 4 > -#endif The #ifndef is there because arch/m68k/include/asm/export.h:1 defines KCRC_ALIGN. You should delete that, too. > diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c > index 4827c5abe5b7..6e6933ae7911 100644 > --- a/scripts/genksyms/genksyms.c > +++ b/scripts/genksyms/genksyms.c > @@ -33,7 +33,7 @@ char *cur_filename; > int in_source_file; > > static int flag_debug, flag_dump_defs, flag_reference, flag_dump_types, > - flag_preserve, flag_warnings, flag_rel_crcs; > + flag_preserve, flag_warnings; > > static int errors; > static int nsyms; > @@ -681,10 +681,7 @@ void export_symbol(const char *name) > fputs(">\n", debugfile); > > /* Used as a linker script. */ ^ Does this comment still apply? > - printf(!flag_rel_crcs ? "__crc_%s = 0x%08lx;\n" : > - "SECTIONS { .rodata : ALIGN(4) { " > - "__crc_%s = .; LONG(0x%08lx); } }\n", > - name, crc); > + printf("__crc_%s = 0x%08lx;\n", name, crc); > } > } > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index eceb3ee7ec06..6aee2401f3ad 100755 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -88,11 +88,6 @@ modpost_link() > gen_initcalls > lds="-T .tmp_initcalls.lds" > > - if is_enabled CONFIG_MODVERSIONS; then > - gen_symversions ^ this is the only caller of gen_symversions, right? Then gen_symversions can be cleaned up, too? > - lds="${lds} -T .tmp_symversions.lds" > - fi > - > # This might take a while, so indicate that we're doing > # an LTO link > info LTO ${1} > @@ -183,6 +178,10 @@ vmlinux_link() > libs="${KBUILD_VMLINUX_LIBS}" > fi > > + if is_enabled CONFIG_MODULES; then > + objs="${objs} .vmlinux.export.o" > + fi > + > if [ "${SRCARCH}" = "um" ]; then > wl=-Wl, > ld="${CC}" > @@ -312,6 +311,7 @@ cleanup() > rm -f vmlinux.o > rm -f .vmlinux.d > rm -f .vmlinux.objs > + rm -f .vmlinux.export.c Probably can drop the `rm -f .tmp_symversions.lds` here, too? -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 06/14] kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS 2022-05-09 17:50 ` Nick Desaulniers @ 2022-05-10 13:03 ` Masahiro Yamada 0 siblings, 0 replies; 33+ messages in thread From: Masahiro Yamada @ 2022-05-10 13:03 UTC (permalink / raw) To: Nick Desaulniers Cc: Linux Kbuild mailing list, Linux Kernel Mailing List, Nathan Chancellor, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, Ard Biesheuvel, Sami Tolvanen, clang-built-linux On Tue, May 10, 2022 at 2:51 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Sun, May 8, 2022 at 12:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h > > index 07a36a874dca..51ce72ce80fa 100644 > > --- a/include/asm-generic/export.h > > +++ b/include/asm-generic/export.h > > @@ -2,6 +2,14 @@ > > #ifndef __ASM_GENERIC_EXPORT_H > > #define __ASM_GENERIC_EXPORT_H > > > > +/* > > + * This comment block is used by fixdep. Please do not remove. > > I don't know much about fixdep. How does that work, if you could summarize? You can find detailed explanation in scripts/basic/fixdep.c In short, it works like this: fixdep parses every source (including headers). If it finds "CONFIG_MODVERSIONS", it adds a dependency on $(wildcard include/config/MODVERSIONS) to the .cmd files. If CONFIG_MODVERSIONS is toggled in Kconfig, it touches include/config/MODVERSIONS. [1] In the next run of Make, all the sources depending on CONFIG_MODVERSIONS will be re-compiled because the timestamp of include/config/MODVERSIONS is up-to-date. [1]: https://github.com/torvalds/linux/blob/v5.17/scripts/kconfig/confdata.c#L141 > > + * > > + * When CONFIG_MODVERSIONS is changed from n to y, all source files having > > + * EXPORT_SYMBOL variants must be re-compiled because genksyms is run as a > > + * side effect of the .o build rule. > > + */ > > + > > #ifndef KSYM_FUNC > > #define KSYM_FUNC(x) x > > #endif > > @@ -12,9 +20,6 @@ > > #else > > #define KSYM_ALIGN 4 > > #endif > > -#ifndef KCRC_ALIGN > > -#define KCRC_ALIGN 4 > > -#endif > > The #ifndef is there because arch/m68k/include/asm/export.h:1 defines > KCRC_ALIGN. You should delete that, too. Nice catch! I will clean it up too. > > > diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c > > index 4827c5abe5b7..6e6933ae7911 100644 > > --- a/scripts/genksyms/genksyms.c > > +++ b/scripts/genksyms/genksyms.c > > @@ -33,7 +33,7 @@ char *cur_filename; > > int in_source_file; > > > > static int flag_debug, flag_dump_defs, flag_reference, flag_dump_types, > > - flag_preserve, flag_warnings, flag_rel_crcs; > > + flag_preserve, flag_warnings; > > > > static int errors; > > static int nsyms; > > @@ -681,10 +681,7 @@ void export_symbol(const char *name) > > fputs(">\n", debugfile); > > > > /* Used as a linker script. */ > > ^ Does this comment still apply? No. From this commit going forward, the genksyms output will not be used as a linker script. 08/14 will delete this comment anyway, but it is possible to remove it in this commit. > > > - printf(!flag_rel_crcs ? "__crc_%s = 0x%08lx;\n" : > > - "SECTIONS { .rodata : ALIGN(4) { " > > - "__crc_%s = .; LONG(0x%08lx); } }\n", > > - name, crc); > > + printf("__crc_%s = 0x%08lx;\n", name, crc); > > } > > } > > > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > > index eceb3ee7ec06..6aee2401f3ad 100755 > > --- a/scripts/link-vmlinux.sh > > +++ b/scripts/link-vmlinux.sh > > @@ -88,11 +88,6 @@ modpost_link() > > gen_initcalls > > lds="-T .tmp_initcalls.lds" > > > > - if is_enabled CONFIG_MODVERSIONS; then > > - gen_symversions > > ^ this is the only caller of gen_symversions, right? Then > gen_symversions can be cleaned up, too? We can keep it in this commit. The follow-up cleaning is done in 07/14. To avoid too big commit, I separated the build flow change and trivial cleanups. > > > - lds="${lds} -T .tmp_symversions.lds" > > - fi > > - > > # This might take a while, so indicate that we're doing > > # an LTO link > > info LTO ${1} > > @@ -183,6 +178,10 @@ vmlinux_link() > > libs="${KBUILD_VMLINUX_LIBS}" > > fi > > > > + if is_enabled CONFIG_MODULES; then > > + objs="${objs} .vmlinux.export.o" > > + fi > > + > > if [ "${SRCARCH}" = "um" ]; then > > wl=-Wl, > > ld="${CC}" > > @@ -312,6 +311,7 @@ cleanup() > > rm -f vmlinux.o > > rm -f .vmlinux.d > > rm -f .vmlinux.objs > > + rm -f .vmlinux.export.c > > Probably can drop the `rm -f .tmp_symversions.lds` here, too? It will be cleaned up by 07/14. > -- > Thanks, > ~Nick Desaulniers -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v4 07/14] kbuild: stop merging *.symversions 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada ` (5 preceding siblings ...) 2022-05-08 19:06 ` [PATCH v4 06/14] kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS Masahiro Yamada @ 2022-05-08 19:06 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 08/14] genksyms: adjust the output format to modpost Masahiro Yamada ` (7 subsequent siblings) 14 siblings, 0 replies; 33+ messages in thread From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada Now modpost reads symbol versions from .*.cmd files. The merged *.symversions are no longer needed. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> Tested-by: Nathan Chancellor <nathan@kernel.org> --- (no changes since v1) scripts/Makefile.build | 21 ++------------------- scripts/link-vmlinux.sh | 15 --------------- 2 files changed, 2 insertions(+), 34 deletions(-) diff --git a/scripts/Makefile.build b/scripts/Makefile.build index ddd9080fc028..dff9220135c4 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -390,17 +390,6 @@ $(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 $(objtree)/scripts/asn1_compiler $(subdir-builtin): $(obj)/%/built-in.a: $(obj)/% ; $(subdir-modorder): $(obj)/%/modules.order: $(obj)/% ; -# combine symversions for later processing -ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y) - cmd_update_lto_symversions = \ - rm -f $@.symversions \ - $(foreach n, $(filter-out FORCE,$^), \ - $(if $(shell test -s $(n).symversions && echo y), \ - ; cat $(n).symversions >> $@.symversions)) -else - cmd_update_lto_symversions = echo >/dev/null -endif - # # Rule to compile a set of .o files into one .a file (without symbol table) # @@ -408,11 +397,8 @@ endif quiet_cmd_ar_builtin = AR $@ cmd_ar_builtin = rm -f $@; $(AR) cDPrST $@ $(real-prereqs) -quiet_cmd_ar_and_symver = AR $@ - cmd_ar_and_symver = $(cmd_update_lto_symversions); $(cmd_ar_builtin) - $(obj)/built-in.a: $(real-obj-y) FORCE - $(call if_changed,ar_and_symver) + $(call if_changed,ar_builtin) # # Rule to create modules.order file @@ -432,16 +418,13 @@ $(obj)/modules.order: $(obj-m) FORCE # # Rule to compile a set of .o files into one .a file (with symbol table) # -quiet_cmd_ar_lib = AR $@ - cmd_ar_lib = $(cmd_update_lto_symversions); $(cmd_ar) $(obj)/lib.a: $(lib-y) FORCE - $(call if_changed,ar_lib) + $(call if_changed,ar) ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),) quiet_cmd_link_multi-m = AR [M] $@ cmd_link_multi-m = \ - $(cmd_update_lto_symversions); \ rm -f $@; \ $(AR) cDPrsT $@ @$(patsubst %.o,%.mod,$@) else diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index 6aee2401f3ad..bc94252e920c 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -56,20 +56,6 @@ gen_initcalls() > .tmp_initcalls.lds } -# If CONFIG_LTO_CLANG is selected, collect generated symbol versions into -# .tmp_symversions.lds -gen_symversions() -{ - info GEN .tmp_symversions.lds - rm -f .tmp_symversions.lds - - for o in ${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}; do - if [ -f ${o}.symversions ]; then - cat ${o}.symversions >> .tmp_symversions.lds - fi - done -} - # Link of vmlinux.o used for section mismatch analysis # ${1} output file modpost_link() @@ -303,7 +289,6 @@ cleanup() rm -f .btf.* rm -f .tmp_System.map rm -f .tmp_initcalls.lds - rm -f .tmp_symversions.lds rm -f .tmp_vmlinux* rm -f System.map rm -f vmlinux -- 2.32.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v4 08/14] genksyms: adjust the output format to modpost 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada ` (6 preceding siblings ...) 2022-05-08 19:06 ` [PATCH v4 07/14] kbuild: stop merging *.symversions Masahiro Yamada @ 2022-05-08 19:06 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 09/14] kbuild: do not create *.prelink.o for Clang LTO or IBT Masahiro Yamada ` (6 subsequent siblings) 14 siblings, 0 replies; 33+ messages in thread From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada Make genksyms output symbol versions in the format modpost expects, so the 'sed' is unneeded. This commit makes *.symversions completely unneeded. I will keep *.symversions in .gitignore and 'make clean' for a while. Otherwise, 'git status' might be surprising. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> Tested-by: Nathan Chancellor <nathan@kernel.org> --- (no changes since v2) Changes in v2: - New patch scripts/Makefile.build | 6 ------ scripts/genksyms/genksyms.c | 3 +-- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/scripts/Makefile.build b/scripts/Makefile.build index dff9220135c4..461998a2ad2b 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -165,16 +165,10 @@ ifdef CONFIG_MODVERSIONS # o modpost will extract versions from that file and create *.c files that will # be compiled and linked to the kernel and/or modules. -genksyms_format := __crc_\(.*\) = \(.*\); - gen_symversions = \ if $(NM) $@ 2>/dev/null | grep -q __ksymtab; then \ $(call cmd_gensymtypes_$(1),$(KBUILD_SYMTYPES),$(@:.o=.symtypes)) \ - > $@.symversions; \ - sed -n 's/$(genksyms_format)/$(pound)SYMVER \1 \2/p' $@.symversions \ >> $(dot-target).cmd; \ - else \ - rm -f $@.symversions; \ fi cmd_gen_symversions_c = $(call gen_symversions,c) diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c index 6e6933ae7911..f5dfdb9d80e9 100644 --- a/scripts/genksyms/genksyms.c +++ b/scripts/genksyms/genksyms.c @@ -680,8 +680,7 @@ void export_symbol(const char *name) if (flag_dump_defs) fputs(">\n", debugfile); - /* Used as a linker script. */ - printf("__crc_%s = 0x%08lx;\n", name, crc); + printf("#SYMVER %s 0x%08lx\n", name, crc); } } -- 2.32.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v4 09/14] kbuild: do not create *.prelink.o for Clang LTO or IBT 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada ` (7 preceding siblings ...) 2022-05-08 19:06 ` [PATCH v4 08/14] genksyms: adjust the output format to modpost Masahiro Yamada @ 2022-05-08 19:06 ` Masahiro Yamada 2022-05-09 23:13 ` Sami Tolvanen 2022-05-08 19:06 ` [PATCH v4 10/14] kbuild: check static EXPORT_SYMBOL* by script instead of modpost Masahiro Yamada ` (5 subsequent siblings) 14 siblings, 1 reply; 33+ messages in thread From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada When CONFIG_LTO_CLANG=y, additional intermediate *.prelink.o is created for each module. Also, objtool is postponed until LLVM bitcode is converted to ELF. CONFIG_X86_KERNEL_IBT works in a similar way to postpone objtool until objects are merged together. This commit stops generating *.prelink.o, so the build flow will look the same with/without LTO. The following figures show how the LTO build currently works, and how this commit is changing it. Current build flow ================== [1] single-object module $(LD) $(CC) +objtool $(LD) foo.c --------------------> foo.o -----> foo.prelink.o -----> foo.ko (LLVM bitcode) (ELF) | | foo.mod.o --/ [2] multi-object module $(LD) $(CC) $(AR) +objtool $(LD) foo1.c -----> foo1.o -----> foo.o -----> foo.prelink.o -----> foo.ko | (archive) (ELF) | foo2.c -----> foo2.o --/ | (LLVM bitcode) foo.mod.o --/ One confusion is foo.o in multi-object module is an archive despite of its suffix. New build flow ============== [1] single-object module Since there is only one object, we do not need to have the LLVM bitcode stage. Use $(CC)+$(LD) to generate an ELF object in one build rule. When LTO is disabled, $(LD) is unneeded because $(CC) produces an ELF object. $(CC)+$(LD)+objtool $(LD) foo.c ------------------------> foo.o -------> foo.ko (ELF) | | foo.mod.o --/ [2] multi-object module Previously, $(AR) was used to combine LLVM bitcode into an archive, but there was no technical reason to do so. This commit just uses $(LD) to combine and convert them into a single ELF object. $(LD) $(CC) +objtool $(LD) foo1.c -------> foo1.o -------> foo.o -------> foo.ko | (ELF) | foo2.c -------> foo2.o ---/ | (LLVM bitcode) foo.mod.o --/ Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> Tested-by: Nathan Chancellor <nathan@kernel.org> --- (no changes since v2) Changes in v2: - replace the chain of $(if ...) with $(and ) scripts/Kbuild.include | 4 +++ scripts/Makefile.build | 58 ++++++++++++--------------------------- scripts/Makefile.lib | 7 ----- scripts/Makefile.modfinal | 5 ++-- scripts/Makefile.modpost | 9 ++---- scripts/mod/modpost.c | 7 ----- 6 files changed, 25 insertions(+), 65 deletions(-) diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index 3514c2149e9d..455a0a6ce12d 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -15,6 +15,10 @@ pound := \# # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o dot-target = $(dir $@).$(notdir $@) +### +# Name of target with a '.tmp_' as filename prefix. foo/bar.o => foo/.tmp_bar.o +tmp-target = $(dir $@).tmp_$(notdir $@) + ### # The temporary file to save gcc -MMD generated dependencies must not # contain a comma diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 461998a2ad2b..838ea5e83174 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -88,10 +88,6 @@ endif targets-for-modules := $(foreach x, o mod $(if $(CONFIG_TRIM_UNUSED_KSYMS), usyms), \ $(patsubst %.o, %.$x, $(filter %.o, $(obj-m)))) -ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),) -targets-for-modules += $(patsubst %.o, %.prelink.o, $(filter %.o, $(obj-m))) -endif - ifdef need-modorder targets-for-modules += $(obj)/modules.order endif @@ -152,8 +148,16 @@ $(obj)/%.ll: $(src)/%.c FORCE # The C file is compiled and updated dependency information is generated. # (See cmd_cc_o_c + relevant part of rule_cc_o_c) +is-single-obj-m = $(and $(part-of-module),$(filter $@, $(obj-m)),y) + +ifdef CONFIG_LTO_CLANG +cmd_ld_single_m = $(if $(is-single-obj-m), ; $(LD) $(ld_flags) -r -o $(tmp-target) $@; mv $(tmp-target) $@) +endif + quiet_cmd_cc_o_c = CC $(quiet_modtag) $@ - cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $< $(cmd_objtool) + cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $< \ + $(cmd_ld_single_m) \ + $(cmd_objtool) ifdef CONFIG_MODVERSIONS # When module versioning is enabled the following steps are executed: @@ -224,21 +228,16 @@ cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(o endif # CONFIG_STACK_VALIDATION -ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),) - -# Skip objtool for LLVM bitcode -$(obj)/%.o: objtool-enabled := - -else # 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory # 'OBJECT_FILES_NON_STANDARD_foo.o := 'y': skip objtool checking for a file # 'OBJECT_FILES_NON_STANDARD_foo.o := 'n': override directory skip for a file -$(obj)/%.o: objtool-enabled = $(if $(filter-out y%, \ - $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y) +is-standard-object = $(if $(filter-out y%, $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y) -endif +delay-objtool := $(or $(CONFIG_LTO_CLANG),$(CONFIG_X86_KERNEL_IBT)) + +$(obj)/%.o: objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y)) ifdef CONFIG_TRIM_UNUSED_KSYMS cmd_gen_ksymdeps = \ @@ -267,24 +266,6 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE $(call if_changed_rule,cc_o_c) $(call cmd,force_checksrc) -ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),) -# Module .o files may contain LLVM bitcode, compile them into native code -# before ELF processing -quiet_cmd_cc_prelink_modules = LD [M] $@ - cmd_cc_prelink_modules = \ - $(LD) $(ld_flags) -r -o $@ \ - --whole-archive $(filter-out FORCE,$^) \ - $(cmd_objtool) - -# objtool was skipped for LLVM bitcode, run it now that we have compiled -# modules into native code -$(obj)/%.prelink.o: objtool-enabled = y -$(obj)/%.prelink.o: part-of-module := y - -$(obj)/%.prelink.o: $(obj)/%.o FORCE - $(call if_changed,cc_prelink_modules) -endif - cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) | \ $(AWK) -v RS='( |\n)' '!x[$$0]++' > $@ @@ -294,7 +275,7 @@ $(obj)/%.mod: FORCE # List module undefined symbols cmd_undefined_syms = $(NM) $< | sed -n 's/^ *U //p' > $@ -$(obj)/%.usyms: $(obj)/%$(mod-prelink-ext).o FORCE +$(obj)/%.usyms: $(obj)/%.o FORCE $(call if_changed,undefined_syms) quiet_cmd_cc_lst_c = MKLST $@ @@ -416,16 +397,11 @@ $(obj)/modules.order: $(obj-m) FORCE $(obj)/lib.a: $(lib-y) FORCE $(call if_changed,ar) -ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),) -quiet_cmd_link_multi-m = AR [M] $@ -cmd_link_multi-m = \ - rm -f $@; \ - $(AR) cDPrsT $@ @$(patsubst %.o,%.mod,$@) -else quiet_cmd_link_multi-m = LD [M] $@ - cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ @$(patsubst %.o,%.mod,$@) -endif + cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ @$(patsubst %.o,%.mod,$@) $(cmd_objtool) +$(multi-obj-m): objtool-enabled := $(delay-objtool) +$(multi-obj-m): part-of-module := y $(multi-obj-m): %.o: %.mod FORCE $(call if_changed,link_multi-m) $(call multi_depend, $(multi-obj-m), .o, -objs -y -m) diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 0453a1904646..f75138385449 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -225,13 +225,6 @@ dtc_cpp_flags = -Wp,-MMD,$(depfile).pre.tmp -nostdinc \ $(addprefix -I,$(DTC_INCLUDE)) \ -undef -D__DTS__ -ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),) -# With CONFIG_LTO_CLANG, .o files in modules might be LLVM bitcode, so we -# need to run LTO to compile them into native code (.lto.o) before further -# processing. -mod-prelink-ext := .prelink -endif - # Useful for describing the dependency of composite objects # Usage: # $(call multi_depend, multi_used_targets, suffix_to_remove, suffix_to_add) diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal index 7f39599e9fae..35100e981f4a 100644 --- a/scripts/Makefile.modfinal +++ b/scripts/Makefile.modfinal @@ -9,7 +9,7 @@ __modfinal: include include/config/auto.conf include $(srctree)/scripts/Kbuild.include -# for c_flags and mod-prelink-ext +# for c_flags include $(srctree)/scripts/Makefile.lib # find all modules listed in modules.order @@ -54,9 +54,8 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \ $(cmd); \ printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:) - # Re-generate module BTFs if either module's .ko or vmlinux changed -$(modules): %.ko: %$(mod-prelink-ext).o %.mod.o scripts/module.lds $(if $(KBUILD_BUILTIN),vmlinux) FORCE +$(modules): %.ko: %.o %.mod.o scripts/module.lds $(if $(KBUILD_BUILTIN),vmlinux) FORCE +$(call if_changed_except,ld_ko_o,vmlinux) ifdef CONFIG_DEBUG_INFO_BTF_MODULES +$(if $(newer-prereqs),$(call cmd,btf_ko)) diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost index 48585c4d04ad..f2ce411acd59 100644 --- a/scripts/Makefile.modpost +++ b/scripts/Makefile.modpost @@ -41,9 +41,6 @@ __modpost: include include/config/auto.conf include $(srctree)/scripts/Kbuild.include -# for mod-prelink-ext -include $(srctree)/scripts/Makefile.lib - MODPOST = scripts/mod/modpost \ $(if $(CONFIG_MODVERSIONS),-m) \ $(if $(CONFIG_MODULE_SRCVERSION_ALL),-a) \ @@ -118,8 +115,6 @@ $(input-symdump): @echo >&2 ' Modules may not have dependencies or modversions.' @echo >&2 ' You may get many unresolved symbol warnings.' -modules := $(sort $(shell cat $(MODORDER))) - # KBUILD_MODPOST_WARN can be set to avoid error out in case of undefined symbols ifneq ($(KBUILD_MODPOST_WARN)$(filter-out $(existing-input-symdump), $(input-symdump)),) MODPOST += -w @@ -128,9 +123,9 @@ endif # Read out modules.order to pass in modpost. # Otherwise, allmodconfig would fail with "Argument list too long". quiet_cmd_modpost = MODPOST $@ - cmd_modpost = sed 's/\.ko$$/$(mod-prelink-ext)\.o/' $< | $(MODPOST) -T - + cmd_modpost = sed 's/ko$$/o/' $< | $(MODPOST) -T - -$(output-symdump): $(MODORDER) $(input-symdump) $(modules:.ko=$(mod-prelink-ext).o) FORCE +$(output-symdump): $(MODORDER) $(input-symdump) FORCE $(call if_changed,modpost) targets += $(output-symdump) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index b46653d46e3d..018527d96680 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -716,10 +716,6 @@ static char *remove_dot(char *s) size_t m = strspn(s + n + 1, "0123456789"); if (m && (s[n + m] == '.' || s[n + m] == 0)) s[n] = 0; - - /* strip trailing .prelink */ - if (strends(s, ".prelink")) - s[strlen(s) - 8] = '\0'; } return s; } @@ -839,9 +835,6 @@ static void read_symbols(const char *modname) /* strip trailing .o */ tmp = NOFAIL(strdup(modname)); tmp[strlen(tmp) - 2] = '\0'; - /* strip trailing .prelink */ - if (strends(tmp, ".prelink")) - tmp[strlen(tmp) - 8] = '\0'; mod = new_module(tmp); free(tmp); } -- 2.32.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v4 09/14] kbuild: do not create *.prelink.o for Clang LTO or IBT 2022-05-08 19:06 ` [PATCH v4 09/14] kbuild: do not create *.prelink.o for Clang LTO or IBT Masahiro Yamada @ 2022-05-09 23:13 ` Sami Tolvanen 0 siblings, 0 replies; 33+ messages in thread From: Sami Tolvanen @ 2022-05-09 23:13 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, LKML, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel On Sun, May 8, 2022 at 12:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > When CONFIG_LTO_CLANG=y, additional intermediate *.prelink.o is created > for each module. Also, objtool is postponed until LLVM bitcode is > converted to ELF. > > CONFIG_X86_KERNEL_IBT works in a similar way to postpone objtool until > objects are merged together. > > This commit stops generating *.prelink.o, so the build flow will look > the same with/without LTO. > > The following figures show how the LTO build currently works, and > how this commit is changing it. > > Current build flow > ================== > > [1] single-object module > > $(LD) > $(CC) +objtool $(LD) > foo.c --------------------> foo.o -----> foo.prelink.o -----> foo.ko > (LLVM bitcode) (ELF) | > | > foo.mod.o --/ > > [2] multi-object module > $(LD) > $(CC) $(AR) +objtool $(LD) > foo1.c -----> foo1.o -----> foo.o -----> foo.prelink.o -----> foo.ko > | (archive) (ELF) | > foo2.c -----> foo2.o --/ | > (LLVM bitcode) foo.mod.o --/ > > One confusion is foo.o in multi-object module is an archive despite of > its suffix. > > New build flow > ============== > > [1] single-object module > > Since there is only one object, we do not need to have the LLVM > bitcode stage. Use $(CC)+$(LD) to generate an ELF object in one > build rule. When LTO is disabled, $(LD) is unneeded because $(CC) > produces an ELF object. > > $(CC)+$(LD)+objtool $(LD) > foo.c ------------------------> foo.o -------> foo.ko > (ELF) | > | > foo.mod.o --/ > > [2] multi-object module > > Previously, $(AR) was used to combine LLVM bitcode into an archive, > but there was no technical reason to do so. > This commit just uses $(LD) to combine and convert them into a single > ELF object. > > $(LD) > $(CC) +objtool $(LD) > foo1.c -------> foo1.o -------> foo.o -------> foo.ko > | (ELF) | > foo2.c -------> foo2.o ---/ | > (LLVM bitcode) foo.mod.o --/ > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> > Tested-by: Nathan Chancellor <nathan@kernel.org> Looks good, thanks for cleaning this up! Reviewed-by: Sami Tolvanen <samitolvanen@google.com> Sami ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v4 10/14] kbuild: check static EXPORT_SYMBOL* by script instead of modpost 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada ` (8 preceding siblings ...) 2022-05-08 19:06 ` [PATCH v4 09/14] kbuild: do not create *.prelink.o for Clang LTO or IBT Masahiro Yamada @ 2022-05-08 19:06 ` Masahiro Yamada 2022-05-09 18:05 ` Nick Desaulniers 2022-05-08 19:06 ` [PATCH v4 11/14] kbuild: make built-in.a rule robust against too long argument error Masahiro Yamada ` (4 subsequent siblings) 14 siblings, 1 reply; 33+ messages in thread From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada The 'static' specifier and EXPORT_SYMBOL() are an odd combination. Commit 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions") tried to detect it, but this check has false negatives. Here is the sample code. Makefile: obj-y += foo1.o foo2.o foo1.c: #include <linux/export.h> static void foo(void) {} EXPORT_SYMBOL(foo); foo2.c: void foo(void) {} foo1.c exports the static symbol 'foo', but modpost cannot catch it because it is fooled by foo2.c, which has a global symbol with the same name. s->is_static is cleared if a global symbol with the same name is found somewhere, but EXPORT_SYMBOL() and the global symbol do not necessarily belong to the same compilation unit. This check should be done per compilation unit, but I do not know how to do it in modpost. modpost runs against vmlinux.o or modules, which merges multiple objects, then forgets their origin. It is true modpost gets access to the lists of all the member objects (.vmlinux.objs and *.mod), but it is impossible to parse individual objects in modpost; they might be LLVM IR instead of ELF when CONFIG_LTO_CLANG=y. Add a simple bash script to parse the output from ${NM}. This works for CONFIG_LTO_CLANG=y because llvm-nm can dump symbols of LLVM bitcode. Revert 15bfc2348d54. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- Changes in v4: - New patch scripts/Makefile.build | 4 ++++ scripts/check-local-export | 48 ++++++++++++++++++++++++++++++++++++++ scripts/mod/modpost.c | 28 +--------------------- 3 files changed, 53 insertions(+), 27 deletions(-) create mode 100755 scripts/check-local-export diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 838ea5e83174..c2a173b3fd60 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -244,9 +244,12 @@ cmd_gen_ksymdeps = \ $(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd endif +cmd_check_local_export = $(srctree)/scripts/check-local-export $@ + define rule_cc_o_c $(call cmd_and_fixdep,cc_o_c) $(call cmd,gen_ksymdeps) + $(call cmd,check_local_export) $(call cmd,checksrc) $(call cmd,checkdoc) $(call cmd,gen_objtooldep) @@ -257,6 +260,7 @@ endef define rule_as_o_S $(call cmd_and_fixdep,as_o_S) $(call cmd,gen_ksymdeps) + $(call cmd,check_local_export) $(call cmd,gen_objtooldep) $(call cmd,gen_symversions_S) endef diff --git a/scripts/check-local-export b/scripts/check-local-export new file mode 100755 index 000000000000..d1721fa63057 --- /dev/null +++ b/scripts/check-local-export @@ -0,0 +1,48 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0-only +# +# Copyright (C) 2022 Masahiro Yamada + +set -e +set -o pipefail + +declare -A symbol_types +declare -a export_symbols + +exit_code=0 + +while read value type name +do + # to avoid error for clang LTO; $name may be empty + if [[ $value = -* && -z $name ]]; then + continue + fi + + # The first field (value) may be empty. If so, fix it up. + if [[ -z $name ]]; then + name=${type} + type=${value} + fi + + # save (name, type) in the associative array + symbol_types[$name]=$type + + # append the exported symbol to the array + if [[ $name == __ksymtab_* ]]; then + export_symbols+=(${name#__ksymtab_}) + fi +done < <(${NM} ${1} 2>/dev/null) + +# Catch error in the process substitution +wait $! + +for name in "${export_symbols[@]}" +do + # nm(3) says "If lowercase, the symbol is usually local" + if [[ ${symbol_types[$name]} =~ [a-z] ]]; then + echo "$@: error: local symbol '${name}' was exported" >&2 + exit_code=1 + fi +done + +exit ${exit_code} diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 018527d96680..fa73ddb6a6cf 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -212,7 +212,6 @@ struct symbol { unsigned int crc; bool crc_valid; bool weak; - bool is_static; /* true if symbol is not global */ bool is_gpl_only; /* exported by EXPORT_SYMBOL_GPL */ char name[]; }; @@ -242,7 +241,7 @@ static struct symbol *alloc_symbol(const char *name) memset(s, 0, sizeof(*s)); strcpy(s->name, name); - s->is_static = true; + return s; } @@ -875,20 +874,6 @@ static void read_symbols(const char *modname) sym_get_data(&info, sym)); } - // check for static EXPORT_SYMBOL_* functions && global vars - for (sym = info.symtab_start; sym < info.symtab_stop; sym++) { - unsigned char bind = ELF_ST_BIND(sym->st_info); - - if (bind == STB_GLOBAL || bind == STB_WEAK) { - struct symbol *s = - find_symbol(remove_dot(info.strtab + - sym->st_name)); - - if (s) - s->is_static = false; - } - } - check_sec_ref(mod, modname, &info); if (!mod->is_vmlinux) { @@ -1318,7 +1303,6 @@ static void read_dump(const char *fname) mod->from_dump = true; } s = sym_add_exported(symname, mod, gpl_only); - s->is_static = false; sym_set_crc(s, crc); sym_update_namespace(symname, namespace); } @@ -1383,7 +1367,6 @@ int main(int argc, char **argv) char *missing_namespace_deps = NULL; char *dump_write = NULL, *files_source = NULL; int opt; - int n; LIST_HEAD(dump_lists); struct dump_list *dl, *dl2; @@ -1459,15 +1442,6 @@ int main(int argc, char **argv) if (sec_mismatch_count && !sec_mismatch_warn_only) error("Section mismatches detected.\n" "Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n"); - for (n = 0; n < SYMBOL_HASH_SIZE; n++) { - struct symbol *s; - - for (s = symbolhash[n]; s; s = s->next) { - if (s->is_static) - error("\"%s\" [%s] is a static EXPORT_SYMBOL\n", - s->name, s->module->name); - } - } if (nr_unresolved > MAX_UNRESOLVED_REPORTS) warn("suppressed %u unresolved symbol warnings because there were too many)\n", -- 2.32.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v4 10/14] kbuild: check static EXPORT_SYMBOL* by script instead of modpost 2022-05-08 19:06 ` [PATCH v4 10/14] kbuild: check static EXPORT_SYMBOL* by script instead of modpost Masahiro Yamada @ 2022-05-09 18:05 ` Nick Desaulniers 2022-05-10 13:53 ` Masahiro Yamada 0 siblings, 1 reply; 33+ messages in thread From: Nick Desaulniers @ 2022-05-09 18:05 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, linux-kernel, Nathan Chancellor, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen On Sun, May 8, 2022 at 12:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > diff --git a/scripts/check-local-export b/scripts/check-local-export > new file mode 100755 > index 000000000000..d1721fa63057 > --- /dev/null > +++ b/scripts/check-local-export > @@ -0,0 +1,48 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# Copyright (C) 2022 Masahiro Yamada > + > +set -e > +set -o pipefail > + > +declare -A symbol_types > +declare -a export_symbols > + > +exit_code=0 > + > +while read value type name > +do > + # to avoid error for clang LTO; $name may be empty > + if [[ $value = -* && -z $name ]]; then > + continue > + fi > + > + # The first field (value) may be empty. If so, fix it up. > + if [[ -z $name ]]; then > + name=${type} > + type=${value} > + fi Consider adding examples of output from NM as comments where you're handling special cases. Aren't BOTH from LTO? The first case is: ---------------- T strncpy while the second is U strncpy IIUC? Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 10/14] kbuild: check static EXPORT_SYMBOL* by script instead of modpost 2022-05-09 18:05 ` Nick Desaulniers @ 2022-05-10 13:53 ` Masahiro Yamada 0 siblings, 0 replies; 33+ messages in thread From: Masahiro Yamada @ 2022-05-10 13:53 UTC (permalink / raw) To: Nick Desaulniers Cc: Linux Kbuild mailing list, Linux Kernel Mailing List, Nathan Chancellor, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen On Tue, May 10, 2022 at 3:05 AM 'Nick Desaulniers' via Clang Built Linux <clang-built-linux@googlegroups.com> wrote: > > On Sun, May 8, 2022 at 12:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > diff --git a/scripts/check-local-export b/scripts/check-local-export > > new file mode 100755 > > index 000000000000..d1721fa63057 > > --- /dev/null > > +++ b/scripts/check-local-export > > @@ -0,0 +1,48 @@ > > +#!/bin/bash > > +# SPDX-License-Identifier: GPL-2.0-only > > +# > > +# Copyright (C) 2022 Masahiro Yamada > > + > > +set -e > > +set -o pipefail > > + > > +declare -A symbol_types > > +declare -a export_symbols > > + > > +exit_code=0 > > + > > +while read value type name > > +do > > + # to avoid error for clang LTO; $name may be empty > > + if [[ $value = -* && -z $name ]]; then > > + continue > > + fi > > + > > + # The first field (value) may be empty. If so, fix it up. > > + if [[ -z $name ]]; then > > + name=${type} > > + type=${value} > > + fi > > Consider adding examples of output from NM as comments where you're > handling special cases. > > Aren't BOTH from LTO? The first case is: > > ---------------- T strncpy For LTO, I see ---------------- t in the llvm-nm output. > > while the second is > > U strncpy Right, this happens for all unresolved symbols. The address part is empty. I will add the output example in the comment block. > IIUC? > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > -- > Thanks, > ~Nick Desaulniers > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdkhcJB8Bnrt51siRefWe%2BZSvHagCs2G011PzkkrD3cxQw%40mail.gmail.com. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v4 11/14] kbuild: make built-in.a rule robust against too long argument error 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada ` (9 preceding siblings ...) 2022-05-08 19:06 ` [PATCH v4 10/14] kbuild: check static EXPORT_SYMBOL* by script instead of modpost Masahiro Yamada @ 2022-05-08 19:06 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 12/14] kbuild: make *.mod " Masahiro Yamada ` (3 subsequent siblings) 14 siblings, 0 replies; 33+ messages in thread From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada Kbuild runs at the top of objtree instead of changing the working directory to subdirectories. I think this design is nice overall but some commands have a scalability issue. The build command of built-in.a is one of them whose length scales with: O(D * N) Here, D is the length of the directory path (i.e. $(obj)/ prefix), N is the number of objects in the Makefile, O() is the big O notation. The deeper directory the Makefile directory is located, the more easily it will hit the too long argument error. We can make it better. Trim the $(obj)/ by Make's builtin function, and restore it by a shell command (sed). With this, the command length scales with: O(D + N) In-tree modules still have some room to the limit (ARG_MAX=2097152), but this is more future-proof for big modules in a deep directory. For example, you can build i915 as builtin (CONFIG_DRM_I915=y) and compare drivers/gpu/drm/i915/.built-in.a.cmd with/without this commit. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> Tested-by: Nathan Chancellor <nathan@kernel.org> --- (no changes since v2) Changes in v2: - New patch scripts/Makefile.build | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/Makefile.build b/scripts/Makefile.build index c2a173b3fd60..8f1a355df7aa 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -374,7 +374,10 @@ $(subdir-modorder): $(obj)/%/modules.order: $(obj)/% ; # quiet_cmd_ar_builtin = AR $@ - cmd_ar_builtin = rm -f $@; $(AR) cDPrST $@ $(real-prereqs) + cmd_ar_builtin = rm -f $@; \ + echo $(patsubst $(obj)/%,%,$(real-prereqs)) | \ + sed -E 's:([^ ]+):$(obj)/\1:g' | \ + xargs $(AR) cDPrST $@ $(obj)/built-in.a: $(real-obj-y) FORCE $(call if_changed,ar_builtin) -- 2.32.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v4 12/14] kbuild: make *.mod rule robust against too long argument error 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada ` (10 preceding siblings ...) 2022-05-08 19:06 ` [PATCH v4 11/14] kbuild: make built-in.a rule robust against too long argument error Masahiro Yamada @ 2022-05-08 19:06 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 13/14] kbuild: add cmd_and_savecmd macro Masahiro Yamada ` (2 subsequent siblings) 14 siblings, 0 replies; 33+ messages in thread From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada Like built-in.a, the command length of the *.mod rule scales with the depth of the directory times the number of objects in the Makefile. Add $(obj)/ by the shell command (awk) instead of by Make's builtin function. In-tree modules still have some room to the limit (ARG_MAX=2097152), but this is more future-proof for big modules in a deep directory. For example, you can build i915 as a module (CONFIG_DRM_I915=m) and compare drivers/gpu/drm/i915/.i915.mod.cmd with/without this commit. The issue is more critical for external modules because the M= path can be very long as Jeff Johnson reported before [1]. [1] https://lore.kernel.org/linux-kbuild/4c02050c4e95e4cb8cc04282695f8404@codeaurora.org/ Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> Tested-by: Nathan Chancellor <nathan@kernel.org> --- (no changes since v2) Changes in v2: - New patch scripts/Makefile.build | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 8f1a355df7aa..f546b5f1f33f 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -270,8 +270,8 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE $(call if_changed_rule,cc_o_c) $(call cmd,force_checksrc) -cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) | \ - $(AWK) -v RS='( |\n)' '!x[$$0]++' > $@ +cmd_mod = echo $(call real-search, $*.o, .o, -objs -y -m) | \ + $(AWK) -v RS='( |\n)' '!x[$$0]++ { print("$(obj)/"$$0) }' > $@ $(obj)/%.mod: FORCE $(call if_changed,mod) -- 2.32.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v4 13/14] kbuild: add cmd_and_savecmd macro 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada ` (11 preceding siblings ...) 2022-05-08 19:06 ` [PATCH v4 12/14] kbuild: make *.mod " Masahiro Yamada @ 2022-05-08 19:06 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 14/14] kbuild: rebuild multi-object modules when objtool is updated Masahiro Yamada 2022-05-09 4:24 ` [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada 14 siblings, 0 replies; 33+ messages in thread From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada, Kees Cook Separate out the command execution part of if_changed, as we did for if_changed_dep. This allows us to reuse it in if_changed_rule. define rule_foo $(call cmd_and_savecmd,foo) $(call cmd,bar) endef Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Kees Cook <keescook@chromium.org> --- Changes in v4: - New. Resent of my previous submission. https://lore.kernel.org/all/20210831074004.3195284-10-masahiroy@kernel.org/ scripts/Kbuild.include | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index 455a0a6ce12d..ece44b735061 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -142,9 +142,11 @@ check-FORCE = $(if $(filter FORCE, $^),,$(warning FORCE prerequisite is missing) if-changed-cond = $(newer-prereqs)$(cmd-check)$(check-FORCE) # Execute command if command has changed or prerequisite(s) are updated. -if_changed = $(if $(if-changed-cond), \ +if_changed = $(if $(if-changed-cond),$(cmd_and_savecmd),@:) + +cmd_and_savecmd = \ $(cmd); \ - printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:) + printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd # Execute the command and also postprocess generated .d dependencies file. if_changed_dep = $(if $(if-changed-cond),$(cmd_and_fixdep),@:) -- 2.32.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v4 14/14] kbuild: rebuild multi-object modules when objtool is updated 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada ` (12 preceding siblings ...) 2022-05-08 19:06 ` [PATCH v4 13/14] kbuild: add cmd_and_savecmd macro Masahiro Yamada @ 2022-05-08 19:06 ` Masahiro Yamada 2022-05-09 4:24 ` [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada 14 siblings, 0 replies; 33+ messages in thread From: Masahiro Yamada @ 2022-05-08 19:06 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen, Masahiro Yamada, Kees Cook, Josh Poimboeuf When CONFIG_LTO_CLANG or CONFIG_X86_KERNEL_IBT is enabled, objtool for multi-object modules is postponed until the objects are linked together. Make sure to re-run objtool and re-link multi-object modules when objtool is updated. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Kees Cook <keescook@chromium.org> Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> --- Changes in v4: - New Resent of my previous submission https://lore.kernel.org/linux-kbuild/20210831074004.3195284-11-masahiroy@kernel.org/ scripts/Makefile.build | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/scripts/Makefile.build b/scripts/Makefile.build index f546b5f1f33f..4e6902e099e8 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -404,13 +404,18 @@ $(obj)/modules.order: $(obj-m) FORCE $(obj)/lib.a: $(lib-y) FORCE $(call if_changed,ar) -quiet_cmd_link_multi-m = LD [M] $@ - cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ @$(patsubst %.o,%.mod,$@) $(cmd_objtool) +quiet_cmd_ld_multi_m = LD [M] $@ + cmd_ld_multi_m = $(LD) $(ld_flags) -r -o $@ @$(patsubst %.o,%.mod,$@) $(cmd_objtool) + +define rule_ld_multi_m + $(call cmd_and_savecmd,ld_multi_m) + $(call cmd,gen_objtooldep) +endef $(multi-obj-m): objtool-enabled := $(delay-objtool) $(multi-obj-m): part-of-module := y $(multi-obj-m): %.o: %.mod FORCE - $(call if_changed,link_multi-m) + $(call if_changed_rule,ld_multi_m) $(call multi_depend, $(multi-obj-m), .o, -objs -y -m) targets := $(filter-out $(PHONY), $(targets)) -- 2.32.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada ` (13 preceding siblings ...) 2022-05-08 19:06 ` [PATCH v4 14/14] kbuild: rebuild multi-object modules when objtool is updated Masahiro Yamada @ 2022-05-09 4:24 ` Masahiro Yamada 2022-05-09 22:19 ` Nathan Chancellor [not found] ` <YnmSCNDBlMMJCbjj@dev-arch.thelio-3990X> 14 siblings, 2 replies; 33+ messages in thread From: Masahiro Yamada @ 2022-05-09 4:24 UTC (permalink / raw) To: Linux Kbuild mailing list Cc: Linux Kernel Mailing List, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen On Mon, May 9, 2022 at 4:09 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > This is the third batch of cleanups in this development cycle. > > Major changes in v4: > - Move static EXPORT_SYMBOL check to a script > - Some refactoring > > Major changes in v3: > > - Generate symbol CRCs as C code, and remove CONFIG_MODULE_REL_CRCS. > > Major changes in v2: > > - V1 did not work with CONFIG_MODULE_REL_CRCS. > I fixed this for v2. > > - Reflect some review comments in v1 > > - Refactor the code more > > - Avoid too long argument error This series is available at git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git lto-cleanup-v4 > > > Masahiro Yamada (14): > modpost: remove left-over cross_compile declaration > modpost: change the license of EXPORT_SYMBOL to bool type > modpost: split the section mismatch checks into section-check.c > modpost: add sym_find_with_module() helper > modpost: extract symbol versions from *.cmd files > kbuild: link symbol CRCs at final link, removing > CONFIG_MODULE_REL_CRCS > kbuild: stop merging *.symversions > genksyms: adjust the output format to modpost > kbuild: do not create *.prelink.o for Clang LTO or IBT > kbuild: check static EXPORT_SYMBOL* by script instead of modpost > kbuild: make built-in.a rule robust against too long argument error > kbuild: make *.mod rule robust against too long argument error > kbuild: add cmd_and_savecmd macro > kbuild: rebuild multi-object modules when objtool is updated > > arch/powerpc/Kconfig | 1 - > arch/s390/Kconfig | 1 - > arch/um/Kconfig | 1 - > include/asm-generic/export.h | 22 +- > include/linux/export-internal.h | 16 + > include/linux/export.h | 30 +- > init/Kconfig | 4 - > kernel/module.c | 10 +- > scripts/Kbuild.include | 10 +- > scripts/Makefile.build | 134 +-- > scripts/Makefile.lib | 7 - > scripts/Makefile.modfinal | 5 +- > scripts/Makefile.modpost | 9 +- > scripts/check-local-export | 48 + > scripts/genksyms/genksyms.c | 18 +- > scripts/link-vmlinux.sh | 33 +- > scripts/mod/Makefile | 2 +- > scripts/mod/modpost.c | 1499 ++++--------------------------- > scripts/mod/modpost.h | 35 +- > scripts/mod/section-check.c | 1222 +++++++++++++++++++++++++ > 20 files changed, 1551 insertions(+), 1556 deletions(-) > create mode 100644 include/linux/export-internal.h > create mode 100755 scripts/check-local-export > create mode 100644 scripts/mod/section-check.c > > -- > 2.32.0 > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20220508190631.2386038-1-masahiroy%40kernel.org. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) 2022-05-09 4:24 ` [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada @ 2022-05-09 22:19 ` Nathan Chancellor [not found] ` <YnmSCNDBlMMJCbjj@dev-arch.thelio-3990X> 1 sibling, 0 replies; 33+ messages in thread From: Nathan Chancellor @ 2022-05-09 22:19 UTC (permalink / raw) To: Masahiro Yamada Cc: Linux Kbuild mailing list, Linux Kernel Mailing List, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen [-- Attachment #1: Type: text/plain, Size: 2520 bytes --] [resending due to mailing list bounces from a large plain text attachment...] On Mon, May 09, 2022 at 01:24:33PM +0900, Masahiro Yamada wrote: > On Mon, May 9, 2022 at 4:09 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > This is the third batch of cleanups in this development cycle. > > > > Major changes in v4: > > - Move static EXPORT_SYMBOL check to a script > > - Some refactoring > > > > Major changes in v3: > > > > - Generate symbol CRCs as C code, and remove CONFIG_MODULE_REL_CRCS. > > > > Major changes in v2: > > > > - V1 did not work with CONFIG_MODULE_REL_CRCS. > > I fixed this for v2. > > > > - Reflect some review comments in v1 > > > > - Refactor the code more > > > > - Avoid too long argument error > > This series is available at > git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git > lto-cleanup-v4 Hi Masahiro, I checked this out and went to run it through my QEMU tests but I see two new errors. Failure #1: In file included from scripts/mod/section-check.c:3: scripts/mod/modpost.h:15:10: fatal error: 'elfconfig.h' file not found #include "elfconfig.h" ^~~~~~~~~~~~~ 1 error generated. I was able to get past that with diff --git a/scripts/mod/Makefile b/scripts/mod/Makefile index ca739c6c68a1..c33b83bfbcad 100644 --- a/scripts/mod/Makefile +++ b/scripts/mod/Makefile @@ -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)/section-check.o: $(obj)/elfconfig.h $(obj)/file2alias.o: $(obj)/$(devicetable-offsets-file) quiet_cmd_elfconfig = MKELF $@ Failure #2: GEN .version CHK include/generated/compile.h GEN .tmp_initcalls.lds LTO vmlinux.o OBJTOOL vmlinux.o MODPOST vmlinux.symvers MODINFO modules.builtin.modinfo GEN modules.builtin LD .tmp_vmlinux.btf ld.lld: error: cannot open .vmlinux.export.o: No such file or directory BTF .btf.vmlinux.bin.o pahole: .tmp_vmlinux.btf: No such file or directory CC .vmlinux.export.c LD .tmp_vmlinux.kallsyms1 ld.lld: error: .btf.vmlinux.bin.o: unknown file type make[1]: *** [Makefile:1159: vmlinux] Error 1 I was not really able to see what is going wrong here. Attached is the configuration that I ran into this with. If you need any other information, please let me know! Cheers, Nathan [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 35483 bytes --] ^ permalink raw reply related [flat|nested] 33+ messages in thread
[parent not found: <YnmSCNDBlMMJCbjj@dev-arch.thelio-3990X>]
* Re: [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) [not found] ` <YnmSCNDBlMMJCbjj@dev-arch.thelio-3990X> @ 2022-05-10 6:55 ` Masahiro Yamada 0 siblings, 0 replies; 33+ messages in thread From: Masahiro Yamada @ 2022-05-10 6:55 UTC (permalink / raw) To: Nathan Chancellor Cc: Linux Kbuild mailing list, Linux Kernel Mailing List, Nick Desaulniers, Nicolas Schier, Peter Zijlstra, linux-modules, linux-s390, linuxppc-dev, clang-built-linux, Ard Biesheuvel, Sami Tolvanen On Tue, May 10, 2022 at 7:13 AM Nathan Chancellor <nathan@kernel.org> wrote: > > On Mon, May 09, 2022 at 01:24:33PM +0900, Masahiro Yamada wrote: > > On Mon, May 9, 2022 at 4:09 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > This is the third batch of cleanups in this development cycle. > > > > > > Major changes in v4: > > > - Move static EXPORT_SYMBOL check to a script > > > - Some refactoring > > > > > > Major changes in v3: > > > > > > - Generate symbol CRCs as C code, and remove CONFIG_MODULE_REL_CRCS. > > > > > > Major changes in v2: > > > > > > - V1 did not work with CONFIG_MODULE_REL_CRCS. > > > I fixed this for v2. > > > > > > - Reflect some review comments in v1 > > > > > > - Refactor the code more > > > > > > - Avoid too long argument error > > > > This series is available at > > git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git > > lto-cleanup-v4 > > Hi Masahiro, > > I checked this out and went to run it through my QEMU tests but I see > two new errors. > > Failure #1: > > In file included from scripts/mod/section-check.c:3: > scripts/mod/modpost.h:15:10: fatal error: 'elfconfig.h' file not found > #include "elfconfig.h" > ^~~~~~~~~~~~~ > 1 error generated. > > I was able to get past that with > > diff --git a/scripts/mod/Makefile b/scripts/mod/Makefile > index ca739c6c68a1..c33b83bfbcad 100644 > --- a/scripts/mod/Makefile > +++ b/scripts/mod/Makefile > @@ -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)/section-check.o: $(obj)/elfconfig.h > $(obj)/file2alias.o: $(obj)/$(devicetable-offsets-file) > Thanks for testing. I will slightly refactor the code as follows. @@ -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 +$(addprefix $(obj)/, $(modpost-objs)): $(obj)/elfconfig.h $(obj)/file2alias.o: $(obj)/$(devicetable-offsets-file) quiet_cmd_elfconfig = MKELF $@ > quiet_cmd_elfconfig = MKELF $@ > > Failure #2: > > GEN .version > CHK include/generated/compile.h > GEN .tmp_initcalls.lds > LTO vmlinux.o > OBJTOOL vmlinux.o > MODPOST vmlinux.symvers > MODINFO modules.builtin.modinfo > GEN modules.builtin > LD .tmp_vmlinux.btf > ld.lld: error: cannot open .vmlinux.export.o: No such file or directory > BTF .btf.vmlinux.bin.o > pahole: .tmp_vmlinux.btf: No such file or directory > CC .vmlinux.export.c > LD .tmp_vmlinux.kallsyms1 > ld.lld: error: .btf.vmlinux.bin.o: unknown file type > make[1]: *** [Makefile:1159: vmlinux] Error 1 > > I was not really able to see what is going wrong here. Attached is the > configuration that I ran into this with. If you need any other > information, please let me know! Ah, OK. This is because .vmlinux.export.o is compiled after gen_btf. I will swap the order in v5. > Cheers, > Nathan -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2022-05-12 4:38 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 01/14] modpost: remove left-over cross_compile declaration Masahiro Yamada
2022-05-09 17:08 ` Nick Desaulniers
2022-05-12 4:35 ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 02/14] modpost: change the license of EXPORT_SYMBOL to bool type Masahiro Yamada
2022-05-12 4:36 ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 03/14] modpost: split the section mismatch checks into section-check.c Masahiro Yamada
2022-05-09 17:19 ` Nick Desaulniers
2022-05-10 6:55 ` Masahiro Yamada
2022-05-11 18:47 ` Nick Desaulniers
2022-05-11 19:27 ` Masahiro Yamada
2022-05-11 19:51 ` Jeff Johnson
2022-05-11 20:01 ` Nick Desaulniers
2022-05-08 19:06 ` [PATCH v4 04/14] modpost: add sym_find_with_module() helper Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 05/14] modpost: extract symbol versions from *.cmd files Masahiro Yamada
2022-05-09 21:52 ` Sami Tolvanen
2022-05-08 19:06 ` [PATCH v4 06/14] kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS Masahiro Yamada
2022-05-09 17:50 ` Nick Desaulniers
2022-05-10 13:03 ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 07/14] kbuild: stop merging *.symversions Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 08/14] genksyms: adjust the output format to modpost Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 09/14] kbuild: do not create *.prelink.o for Clang LTO or IBT Masahiro Yamada
2022-05-09 23:13 ` Sami Tolvanen
2022-05-08 19:06 ` [PATCH v4 10/14] kbuild: check static EXPORT_SYMBOL* by script instead of modpost Masahiro Yamada
2022-05-09 18:05 ` Nick Desaulniers
2022-05-10 13:53 ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 11/14] kbuild: make built-in.a rule robust against too long argument error Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 12/14] kbuild: make *.mod " Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 13/14] kbuild: add cmd_and_savecmd macro Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 14/14] kbuild: rebuild multi-object modules when objtool is updated Masahiro Yamada
2022-05-09 4:24 ` [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada
2022-05-09 22:19 ` Nathan Chancellor
[not found] ` <YnmSCNDBlMMJCbjj@dev-arch.thelio-3990X>
2022-05-10 6:55 ` Masahiro Yamada
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox