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@infradead.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;
+ }
+ }
+
+ 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);On Fri, May 2, 2025 at 11:26 PM Peter Zijlstra <peterz@infradead.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@infradead.org>
> ---
> 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);
>
>
I believe this code is wrong because "make nsdeps" would
incorrectly add MOULDE_IMPORT_NS().
When MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS=y,
EXPORT_SYMBOL_NS(foo, "module:bar") can be used by
any module or not.
That is not what we have decided yet.
At least, MODULE_IMPORT_NS("module:bar");
does not solve the issue at all.
--
Best Regards
Masahiro Yamada
On Sat, May 17, 2025 at 4:19 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Fri, May 2, 2025 at 11:26 PM Peter Zijlstra <peterz@infradead.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@infradead.org>
> > ---
>
>
> > 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);
> >
> >
>
>
> I believe this code is wrong because "make nsdeps" would
> incorrectly add MOULDE_IMPORT_NS().
>
>
> When MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS=y,
> EXPORT_SYMBOL_NS(foo, "module:bar") can be used by
> any module or not.
> That is not what we have decided yet.
>
> At least, MODULE_IMPORT_NS("module:bar");
> does not solve the issue at all.
Peter is not responding to my feedback.
I will apply this and fix the code by myself on top of that.
--
Best Regards
Masahiro Yamada
On Fri, May 2, 2025 at 11:26 PM Peter Zijlstra <peterz@infradead.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@infradead.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
On 5/2/25 16:12, Peter Zijlstra 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@infradead.org>
Sorry, I thought this was already reviewed from the modules perspective,
but I'll make it explicit.
Looks ok to me, besides the already mentioned "strsmp" typo. I can fix
it when picking up the series.
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
--
Thanks,
Petr
On Fri, May 2, 2025 at 11:26 PM Peter Zijlstra <peterz@infradead.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@infradead.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);
Apparently, you did not spend time for per-commit compile testing.
I believe it is a typo of "strcmp()".
I got this build error.
../kernel/module/main.c: In function 'verify_module_namespace':
../kernel/module/main.c:1091:17: error: implicit declaration of
function 'strsmp'; did you mean 'strsep'?
[-Wimplicit-function-declaration]
1091 | !strsmp(namespace + strlen(prefix), modname);
| ^~~~~~
| strsep
--
Best Regards
Masahiro Yamada
© 2016 - 2025 Red Hat, Inc.