Message-ID: <CAK7LNAT9wCZ5fenykaBsor-CwOuUXFdg2Fe1eGe_y7qR2X2Ysg@mail.gmail.com>
Date: Sat, 17 May 2025 15:56:03 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: mcgrof@...nel.org, x86@...nel.org, hpa@...or.com, petr.pavlu@...e.com, 
	samitolvanen@...gle.com, da.gomez@...sung.com, nathan@...nel.org, 
	nicolas@...sle.eu, linux-kernel@...r.kernel.org, 
	linux-modules@...r.kernel.org, linux-kbuild@...r.kernel.org, 
	hch@...radead.org, gregkh@...uxfoundation.org, roypat@...zon.co.uk
Subject: Re: [PATCH v3 2/5] module: Add module specific symbol namespace support

On Fri, May 2, 2025 at 11:26 PM Peter Zijlstra <peterz@...radead.org> wrote:
>
> Designate the "module:${modname}" symbol namespace to mean: 'only
> export to the named module'.
>
> Notably, explicit imports of anything in the "module:" space is
> forbidden.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
>  kernel/module/main.c  |   33 +++++++++++++++++++++++++++++++--
>  scripts/mod/modpost.c |   11 ++++++++++-
>  2 files changed, 41 insertions(+), 3 deletions(-)
>
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -1083,6 +1083,14 @@ static char *get_modinfo(const struct lo
>         return get_next_modinfo(info, tag, NULL);
>  }
>
> +static bool verify_module_namespace(const char *namespace, const char *modname)
> +{
> +       const char *prefix = "module:";
> +
> +       return strstarts(namespace, prefix) &&
> +              !strsmp(namespace + strlen(prefix), modname);
> +}
> +
>  static int verify_namespace_is_imported(const struct load_info *info,
>                                         const struct kernel_symbol *sym,
>                                         struct module *mod)
> @@ -1092,6 +1100,10 @@ static int verify_namespace_is_imported(
>
>         namespace = kernel_symbol_namespace(sym);
>         if (namespace && namespace[0]) {
> +
> +               if (verify_module_namespace(namespace, mod->name))
> +                       return 0;
> +
>                 for_each_modinfo_entry(imported_namespace, info, "import_ns") {
>                         if (strcmp(namespace, imported_namespace) == 0)
>                                 return 0;
> @@ -1659,15 +1671,30 @@ static void module_license_taint_check(s
>         }
>  }
>
> -static void setup_modinfo(struct module *mod, struct load_info *info)
> +static int setup_modinfo(struct module *mod, struct load_info *info)
>  {
>         const struct module_attribute *attr;
> +       char *imported_namespace;
>         int i;
>
>         for (i = 0; (attr = modinfo_attrs[i]); i++) {
>                 if (attr->setup)
>                         attr->setup(mod, get_modinfo(info, attr->attr.name));
>         }
> +
> +       for_each_modinfo_entry(imported_namespace, info, "import_ns") {
> +               /*
> +                * 'module:' prefixed namespaces are implicit, disallow
> +                * explicit imports.
> +                */
> +               if (strstarts(imported_namespace, "module:")) {
> +                       pr_err("%s: module tries to import module namespace: %s\n",
> +                              mod->name, imported_namespace);
> +                       return -EPERM;
> +               }


It is also possible to check this at compile-time, i.e., in modpost.

Does it make sense to check this at compile-time instead of
run-time?

(or check it both at compile-time and build-time?)



diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index c9ff4db26edb..30c8d8062792 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1597,9 +1597,14 @@ static void read_symbols(const char *modname)

                for (namespace = get_modinfo(&info, "import_ns");
                     namespace;
-                    namespace = get_next_modinfo(&info, "import_ns",
namespace))
+                    namespace = get_next_modinfo(&info, "import_ns",
namespace)) {
+                       if (strstarts(namespace, "module:"))
+                               error("%s: tries to import module
namespace \"%s\".\n");
+
                        add_namespace(&mod->imported_namespaces, namespace);

+               }
+
                if (!get_modinfo(&info, "description"))
                        warn("missing MODULE_DESCRIPTION() in %s\n", modname);

        }














> +       }
> +
> +       return 0;
>  }
>
>  static void free_modinfo(struct module *mod)
> @@ -3335,7 +3362,9 @@ static int load_module(struct load_info
>                 goto free_unload;
>
>         /* Set up MODINFO_ATTR fields */
> -       setup_modinfo(mod, info);
> +       err = setup_modinfo(mod, info);
> +       if (err)
> +               goto free_modinfo;
>
>         /* Fix up syms, so that st_value is a pointer to location. */
>         err = simplify_symbols(mod, info);
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1682,6 +1682,14 @@ void buf_write(struct buffer *buf, const
>         buf->pos += len;
>  }
>
> +static bool verify_module_namespace(const char *namespace, const char *modname)
> +{
> +       const char *prefix = "module:";
> +
> +       return strstarts(namespace, prefix) &&
> +              !strcmp(namespace + strlen(prefix), modname);
> +}
> +
>  static void check_exports(struct module *mod)
>  {
>         struct symbol *s, *exp;
> @@ -1709,7 +1717,8 @@ static void check_exports(struct module
>
>                 basename = get_basename(mod->name);
>
> -               if (!contains_namespace(&mod->imported_namespaces, exp->namespace)) {
> +               if (!verify_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

Powered by blists - more mailing lists