| Message ID | 20241202150810.496784892@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | module: Strict per-modname namespaces | expand |
On Tue, Dec 3, 2024 at 12:11 AM Peter Zijlstra <peterz@infradead.org> wrote: > > Instead of only accepting "MODULE_${name}", extend it with a comma > separated list of module names and add tail glob support. > > That is, something like: "MODULE_foo-*,bar" is now possible. > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > kernel/module/main.c | 39 ++++++++++++++++++++++++++++++++++----- > scripts/mod/modpost.c | 40 ++++++++++++++++++++++++++++++++++------ > 2 files changed, 68 insertions(+), 11 deletions(-) > > --- a/kernel/module/main.c > +++ b/kernel/module/main.c > @@ -1061,6 +1061,38 @@ static char *get_modinfo(const struct lo > return get_next_modinfo(info, tag, NULL); > } > > +/* > + * @namespace ~= "MODULE_foo-*,bar", match @modname to 'foo-*' or 'bar' Please add a proper kdoc style comment block. Rather than an underscore, I prefer a colon, which rarely appears in file names. How about "module:foo-*,bar" instead of "MODULE_foo-*,bar" ? > + */ > +static bool verify_module_namespace(const char *namespace, const char *modname) > +{ > + size_t len, modlen = strlen(modname); > + const char *sep; > + bool glob; > + > + if (strncmp(namespace, "MODULE_", 7) != 0) > + return false; > + > + for (namespace += 7; *namespace; namespace = sep) { > + sep = strchrnul(namespace, ','); > + len = sep - namespace; > + > + glob = false; > + if (sep[-1] == '*') { > + len--; > + glob = true; > + } Why only limited to the trailing wildcard? Did you consider using glob_match()? > + > + if (*sep) > + sep++; > + > + if (strncmp(namespace, modname, len) == 0 && (glob || len == modlen)) > + return true; > + } > + > + return false; > +} > + > static int verify_namespace_is_imported(const struct load_info *info, > const struct kernel_symbol *sym, > struct module *mod) > @@ -1070,11 +1102,8 @@ static int verify_namespace_is_imported( > > namespace = kernel_symbol_namespace(sym); > if (namespace && namespace[0]) { > - /* > - * Implicitly import MODULE_${mod->name} namespace. > - */ > - if (strncmp(namespace, "MODULE_", 7) == 0 && > - strcmp(namespace+7, mod->name) == 0) > + > + if (verify_module_namespace(namespace, mod->name)) > return 0; > > for_each_modinfo_entry(imported_namespace, info, "import_ns") { > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -1565,7 +1565,6 @@ static const char *mod_basename(const ch > > static void read_symbols(const char *modname) > { > - char module_namespace[MODULE_NAME_LEN + 8]; > const char *symname; > char *version; > char *license; > @@ -1601,10 +1600,6 @@ static void read_symbols(const char *mod > namespace = get_next_modinfo(&info, "import_ns", namespace)) > add_namespace(&mod->imported_namespaces, namespace); > > - snprintf(module_namespace, sizeof(module_namespace), "MODULE_%s", > - mod_basename(mod->name)); > - add_namespace(&mod->imported_namespaces, module_namespace); > - > if (extra_warn && !get_modinfo(&info, "description")) > warn("missing MODULE_DESCRIPTION() in %s\n", modname); > } > @@ -1687,6 +1682,38 @@ void buf_write(struct buffer *buf, const > buf->pos += len; > } > > +/* > + * @namespace ~= "MODULE_foo-*,bar", match @modname to 'foo-*' or 'bar' Same comment as in kernel/module/main.c > + */ > +static bool module_namespace(const char *namespace, const char *modname) > +{ > + size_t len, modlen = strlen(modname); > + const char *sep; > + bool glob; > + > + if (strncmp(namespace, "MODULE_", 7) != 0) > + return false; If you see the existing code closely, strstarts() helper is used everywhere. I would write like this: static const char *prefix = "module:"; if (!strstarts(namespace, prefix)) return false; > + > + for (namespace += 7; *namespace; namespace = sep) { I tend to avoid the magic number, 7. Just like this: https://github.com/torvalds/linux/blob/v6.14-rc1/scripts/mod/file2alias.c#L1494 > + sep = strchrnul(namespace, ','); > + len = sep - namespace; > + > + glob = false; > + if (sep[-1] == '*') { > + len--; > + glob = true; > + } > + > + if (*sep) > + sep++; > + > + if (strncmp(namespace, modname, len) == 0 && (glob || len == modlen)) > + return true; > + } Same as kernel/module/main.c. If you see around line 700 in this file, you will find fnmatch() is already used for glob matching. > + > + return false; > +} > + > static void check_exports(struct module *mod) > { > struct symbol *s, *exp; > @@ -1714,7 +1741,8 @@ static void check_exports(struct module > > basename = mod_basename(mod->name); > > - if (!contains_namespace(&mod->imported_namespaces, exp->namespace)) { > + if (!module_namespace(exp->namespace, basename) && > + !contains_namespace(&mod->imported_namespaces, exp->namespace)) { > modpost_log(!allow_missing_ns_imports, > "module %s uses symbol %s from namespace %s, but does not import it.\n", > basename, exp->name, exp->namespace); > > -- Best Regards Masahiro Yamada
On Tue, Dec 3, 2024 at 12:11 AM Peter Zijlstra <peterz@infradead.org> wrote: > > Instead of only accepting "MODULE_${name}", extend it with a comma > separated list of module names and add tail glob support. > > That is, something like: "MODULE_foo-*,bar" is now possible. > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > kernel/module/main.c | 39 ++++++++++++++++++++++++++++++++++----- > scripts/mod/modpost.c | 40 ++++++++++++++++++++++++++++++++++------ > 2 files changed, 68 insertions(+), 11 deletions(-) > > static void check_exports(struct module *mod) > { > struct symbol *s, *exp; > @@ -1714,7 +1741,8 @@ static void check_exports(struct module > > basename = mod_basename(mod->name); > > - if (!contains_namespace(&mod->imported_namespaces, exp->namespace)) { > + if (!module_namespace(exp->namespace, basename) && > + !contains_namespace(&mod->imported_namespaces, exp->namespace)) { > modpost_log(!allow_missing_ns_imports, > "module %s uses symbol %s from namespace %s, but does not import it.\n", > basename, exp->name, exp->namespace); > When module_namespace() fails, the following error message is shown: ERROR: modpost: module bar uses symbol foo from namespace MODULE_kvm, but does not import it. We get a hint from this error message, but the solution is not MODULE_IMPORT_NS(); 'make nsdeps' will try to solve this by adding MODULE_IMPORT_NS("MODULE_kvm"); Apparently, this does not work. Not sure if you want to solve the issue.
--- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1061,6 +1061,38 @@ static char *get_modinfo(const struct lo return get_next_modinfo(info, tag, NULL); } +/* + * @namespace ~= "MODULE_foo-*,bar", match @modname to 'foo-*' or 'bar' + */ +static bool verify_module_namespace(const char *namespace, const char *modname) +{ + size_t len, modlen = strlen(modname); + const char *sep; + bool glob; + + if (strncmp(namespace, "MODULE_", 7) != 0) + return false; + + for (namespace += 7; *namespace; namespace = sep) { + sep = strchrnul(namespace, ','); + len = sep - namespace; + + glob = false; + if (sep[-1] == '*') { + len--; + glob = true; + } + + if (*sep) + sep++; + + if (strncmp(namespace, modname, len) == 0 && (glob || len == modlen)) + return true; + } + + return false; +} + static int verify_namespace_is_imported(const struct load_info *info, const struct kernel_symbol *sym, struct module *mod) @@ -1070,11 +1102,8 @@ static int verify_namespace_is_imported( namespace = kernel_symbol_namespace(sym); if (namespace && namespace[0]) { - /* - * Implicitly import MODULE_${mod->name} namespace. - */ - if (strncmp(namespace, "MODULE_", 7) == 0 && - strcmp(namespace+7, mod->name) == 0) + + if (verify_module_namespace(namespace, mod->name)) return 0; for_each_modinfo_entry(imported_namespace, info, "import_ns") { --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1565,7 +1565,6 @@ static const char *mod_basename(const ch static void read_symbols(const char *modname) { - char module_namespace[MODULE_NAME_LEN + 8]; const char *symname; char *version; char *license; @@ -1601,10 +1600,6 @@ static void read_symbols(const char *mod namespace = get_next_modinfo(&info, "import_ns", namespace)) add_namespace(&mod->imported_namespaces, namespace); - snprintf(module_namespace, sizeof(module_namespace), "MODULE_%s", - mod_basename(mod->name)); - add_namespace(&mod->imported_namespaces, module_namespace); - if (extra_warn && !get_modinfo(&info, "description")) warn("missing MODULE_DESCRIPTION() in %s\n", modname); } @@ -1687,6 +1682,38 @@ void buf_write(struct buffer *buf, const buf->pos += len; } +/* + * @namespace ~= "MODULE_foo-*,bar", match @modname to 'foo-*' or 'bar' + */ +static bool module_namespace(const char *namespace, const char *modname) +{ + size_t len, modlen = strlen(modname); + const char *sep; + bool glob; + + if (strncmp(namespace, "MODULE_", 7) != 0) + return false; + + for (namespace += 7; *namespace; namespace = sep) { + sep = strchrnul(namespace, ','); + len = sep - namespace; + + glob = false; + if (sep[-1] == '*') { + len--; + glob = true; + } + + if (*sep) + sep++; + + if (strncmp(namespace, modname, len) == 0 && (glob || len == modlen)) + return true; + } + + return false; +} + static void check_exports(struct module *mod) { struct symbol *s, *exp; @@ -1714,7 +1741,8 @@ static void check_exports(struct module basename = mod_basename(mod->name); - if (!contains_namespace(&mod->imported_namespaces, exp->namespace)) { + if (!module_namespace(exp->namespace, basename) && + !contains_namespace(&mod->imported_namespaces, exp->namespace)) { modpost_log(!allow_missing_ns_imports, "module %s uses symbol %s from namespace %s, but does not import it.\n", basename, exp->name, exp->namespace);
Instead of only accepting "MODULE_${name}", extend it with a comma separated list of module names and add tail glob support. That is, something like: "MODULE_foo-*,bar" is now possible. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- kernel/module/main.c | 39 ++++++++++++++++++++++++++++++++++----- scripts/mod/modpost.c | 40 ++++++++++++++++++++++++++++++++++------ 2 files changed, 68 insertions(+), 11 deletions(-)