Currently, enum export is tristate, but export_unknown does not make
sense in any way.

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. The variable name just happens to start with
"__ksymtab_". Do not call sym_add_exported() in this case.

__ksymtab_* is internally by EXPORT_SYMBOL(_GPL) but somebody may
directly define a global variable with a such name, like this:

   int __ksymtab_foo;

Presumably, there is no practical issue for this, but there is no good
reason to use such a weird name.

This commit adds a new warning for such a case:

    WARNING: modpost: __ksymtab_foo: Please consider renaming. Variables 
starting with "__ksymtab_" is for internal use.

With pointless export_unknown removed, the license type of exported
symbols is boolean (EXPORT_SYMBOL or EXPORT_SYMBOL_GPL).

I renamed the field name to is_gpl_only. If it is true, only GPL-compat
modules can use it.

Signed-off-by: Masahiro Yamada <masahi...@kernel.org>
---

Changes in v3:
  - New patch

 scripts/mod/modpost.c | 108 +++++++++++++-----------------------------
 1 file changed, 32 insertions(+), 76 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index a55fa2b88a9a..ebd80c77fa03 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -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[];
 };
 
@@ -321,34 +315,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)
 {
@@ -379,18 +345,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);
@@ -410,7 +364,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);
 
@@ -422,7 +376,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);
 
@@ -694,8 +648,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_")) {
@@ -729,12 +681,18 @@ 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);
+                       else
+                               warn("%s: Please consider renaming. Variables 
starting with \"__ksymtab_\" is for internal use.\n",
+                                    symname);
                }
                if (strcmp(symname, "init_module") == 0)
                        mod->has_init = true;
@@ -2146,20 +2104,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;
@@ -2198,14 +2142,15 @@ 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);
        }
 
        list_for_each_entry(s, &mod->exported_symbols, list) {
                if (s->is_static)
-                       error("\"%s\" [%s] is a static %s\n",
-                             s->name, mod->name, export_str(s->export));
+                       error("\"%s\" [%s] is a static EXPORT_SYMBOL\n",
+                             s->name, mod->name);
        }
 }
 
@@ -2429,6 +2374,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;
@@ -2446,12 +2392,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 for %s. skip", export, 
symname);
+                       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);
@@ -2473,9 +2429,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 ?: "");
                }
        }
-- 
2.32.0


_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

Reply via email to