From patchwork Sun May 8 19:06:19 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Masahiro Yamada X-Patchwork-Id: 12842698 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9CEFBC4167D for ; Sun, 8 May 2022 19:18:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230250AbiEHTWQ (ORCPT ); Sun, 8 May 2022 15:22:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38882 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230420AbiEHTQ4 (ORCPT ); Sun, 8 May 2022 15:16:56 -0400 Received: from conuserg-09.nifty.com (conuserg-09.nifty.com [210.131.2.76]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 736D1C67; Sun, 8 May 2022 12:11:17 -0700 (PDT) Received: from grover.sesame (133-32-177-133.west.xps.vectant.ne.jp [133.32.177.133]) (authenticated) by conuserg-09.nifty.com with ESMTP id 248J8qSP030019; Mon, 9 May 2022 04:08:55 +0900 DKIM-Filter: OpenDKIM Filter v2.10.3 conuserg-09.nifty.com 248J8qSP030019 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nifty.com; s=dec2015msa; t=1652036935; bh=Ql9G6KPWQ5/mHxdBx/eeInIlMujjp41xuyJMJCRCA5c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ztPH/8k6+i1rw3/PSuikV9E/KpQSkWQKS8jKTSuXctYml6nGCgtXN3Lw+MjReDkKu qs1D7oG9NkckiJKqCktbIpyKaUoKCGluQ2sxtRyQ/u5EBI4IDFOp/qLu6S4IkyULV6 ylX4XYU7bDSkky+1anBK8kmB3hjcDeJtHgiM6tHpyTNCk5IaANaQFqTak25OFg9RwU 1Jeh6LXyFV6TXnxVNwA+HPkLV+nCyVPq7JnHcXthPFqp21Jt8EvQCN3CQke06hhxIN Q5L7vyEjIlKFuAW/dJrN2SuJDf/cfcE4vrIdWQ8YPpsFz6U5GUwN0nSccJL3zQbjZk LNvgWpicNgwIg== X-Nifty-SrcIP: [133.32.177.133] From: Masahiro Yamada To: linux-kbuild@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Nathan Chancellor , Nick Desaulniers , Nicolas Schier , Peter Zijlstra , linux-modules@vger.kernel.org, linux-s390@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, clang-built-linux@googlegroups.com, Ard Biesheuvel , Sami Tolvanen , Masahiro Yamada Subject: [PATCH v4 02/14] modpost: change the license of EXPORT_SYMBOL to bool type Date: Mon, 9 May 2022 04:06:19 +0900 Message-Id: <20220508190631.2386038-3-masahiroy@kernel.org> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20220508190631.2386038-1-masahiroy@kernel.org> References: <20220508190631.2386038-1-masahiroy@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: 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 Reviewed-by: Nicolas Schier Tested-by: Nathan Chancellor --- 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); } }