On Tue, Dec 3, 2024 at 12:11 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> ---
> 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
