| Message ID | 20220508190631.2386038-3-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) | expand |
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.
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); } }
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(-)