[PATCH v3 0/5] module: Strict per-modname namespaces

[PATCH v3 0/5] module: Strict per-modname namespaces

[Peter Zijlstra]

Hi!

Implement means for exports to be available to an explicit list of named
modules. By explicitly limiting the usage of certain exports, the abuse
potential/risk is greatly reduced.

Changes since v2:

 - switch to "module:" prefix (Masahiro)
 - removed some patch noise (Masahiro)
 - strstarts() and strlen() usage for prefixes (Masahiro)
 - simpler ___EXPORT_SYMBOL() changes (Masahiro)

Not making using of glob_match() / fnmatch(); this would result in more
complicated code for very little gain.

[PATCH v3 1/5] modpost: Use for() loop

[Peter Zijlstra]

Slight cleanup by using a for() loop instead of while(). This makes it
clearer what is the iteration and what is the actual work done.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 scripts/mod/modpost.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1595,12 +1595,10 @@ static void read_symbols(const char *mod
 			license = get_next_modinfo(&info, "license", license);
 		}
 
-		namespace = get_modinfo(&info, "import_ns");
-		while (namespace) {
+		for (namespace = get_modinfo(&info, "import_ns");
+		     namespace;
+		     namespace = get_next_modinfo(&info, "import_ns", namespace))
 			add_namespace(&mod->imported_namespaces, namespace);
-			namespace = get_next_modinfo(&info, "import_ns",
-						     namespace);
-		}
 
 		if (!get_modinfo(&info, "description"))
 			warn("missing MODULE_DESCRIPTION() in %s\n", modname);

[PATCH v3 2/5] module: Add module specific symbol namespace support

[Peter Zijlstra]

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);

[PATCH v3 3/5] module: Extend the MODULE_ namespace parsing

[Peter Zijlstra]

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  |   36 ++++++++++++++++++++++++++++++++++--
 scripts/mod/modpost.c |   36 ++++++++++++++++++++++++++++++++++--
 2 files changed, 68 insertions(+), 4 deletions(-)

--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1083,12 +1083,44 @@ static char *get_modinfo(const struct lo
 	return get_next_modinfo(info, tag, NULL);
 }
 
+/**
+ * verify_module_namespace() - does @modname have access to this symbol's @namespace
+ * @namespace: export symbol namespace
+ * @modname: module name
+ *
+ * If @namespace is prefixed with "module:" to indicate it is a module namespace
+ * then test if @modname matches any of the comma separated patterns.
+ *
+ * The patterns only support tail-glob.
+ */
 static bool verify_module_namespace(const char *namespace, const char *modname)
 {
+	size_t len, modlen = strlen(modname);
 	const char *prefix = "module:";
+	const char *sep;
+	bool glob;
 
-	return strstarts(namespace, prefix) &&
-	       !strsmp(namespace + strlen(prefix), modname);
+	if (!strstarts(namespace, prefix))
+		return false;
+
+	for (namespace += strlen(prefix); *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,
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1682,12 +1682,44 @@ void buf_write(struct buffer *buf, const
 	buf->pos += len;
 }
 
+/**
+ * verify_module_namespace() - does @modname have access to this symbol's @namespace
+ * @namespace: export symbol namespace
+ * @modname: module name
+ *
+ * If @namespace is prefixed with "module:" to indicate it is a module namespace
+ * then test if @modname matches any of the comma separated patterns.
+ *
+ * The patterns only support tail-glob.
+ */
 static bool verify_module_namespace(const char *namespace, const char *modname)
 {
+	size_t len, modlen = strlen(modname);
 	const char *prefix = "module:";
+	const char *sep;
+	bool glob;
 
-	return strstarts(namespace, prefix) &&
-	       !strcmp(namespace + strlen(prefix), modname);
+	if (!strstarts(namespace, prefix))
+		return false;
+
+	for (namespace += strlen(prefix); *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)

[PATCH v3 4/5] module: Account for the build time module name mangling

[Peter Zijlstra]

Sean noted that scripts/Makefile.lib:name-fix-token rule will mangle
the module name with s/-/_/g.

Since this happens late in the build, only the kernel needs to bother
with this, the modpost tool still sees the original name.

Reported-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Tested-by: Sean Christopherson <seanjc@google.com>
---
 kernel/module/main.c |   26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -170,6 +170,30 @@ static inline void add_taint_module(stru
 }
 
 /*
+ * Like strncmp(), except s/-/_/g as per scripts/Makefile.lib:name-fix-token rule.
+ */
+static int mod_strncmp(const char *str_a, const char *str_b, size_t n)
+{
+	for (int i = 0; i < n; i++) {
+		char a = str_a[i];
+		char b = str_b[i];
+		int d;
+
+		if (a == '-') a = '_';
+		if (b == '-') b = '_';
+
+		d = a - b;
+		if (d)
+			return d;
+
+		if (!a)
+			break;
+	}
+
+	return 0;
+}
+
+/*
  * A thread that wants to hold a reference to a module only while it
  * is running can call this to safely exit.
  */
@@ -1116,7 +1140,7 @@ static bool verify_module_namespace(cons
 		if (*sep)
 			sep++;
 
-		if (strncmp(namespace, modname, len) == 0 && (glob || len == modlen))
+		if (mod_strncmp(namespace, modname, len) == 0 && (glob || len == modlen))
 			return true;
 	}
 

[PATCH v3 5/5] module: Provide EXPORT_SYMBOL_GPL_FOR_MODULES() helper

[Peter Zijlstra]

Helper macro to more easily limit the export of a symbol to a given
list of modules.

Eg:

  EXPORT_SYMBOL_GPL_FOR_MODULES(preempt_notifier_inc, "kvm");

will limit the use of said function to kvm.ko, any other module trying
to use this symbol will refure to load (and get modpost build
failures).

Requested-by: Masahiro Yamada <masahiroy@kernel.org>
Requested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 Documentation/core-api/symbol-namespaces.rst |   22 ++++++++++++++++++++++
 include/linux/export.h                       |   12 ++++++++++--
 2 files changed, 32 insertions(+), 2 deletions(-)

--- a/Documentation/core-api/symbol-namespaces.rst
+++ b/Documentation/core-api/symbol-namespaces.rst
@@ -28,6 +28,9 @@ kernel. As of today, modules that make u
 are required to import the namespace. Otherwise the kernel will, depending on
 its configuration, reject loading the module or warn about a missing import.
 
+Additionally, it is possible to put symbols into a module namespace, strictly
+limiting which modules are allowed to use these symbols.
+
 2. How to define Symbol Namespaces
 ==================================
 
@@ -83,6 +86,22 @@ A second option to define the default na
 within the corresponding compilation unit before the #include for
 <linux/export.h>. Typically it's placed before the first #include statement.
 
+2.3 Using the EXPORT_SYMBOL_GPL_FOR_MODULES() macro
+===================================================
+
+Symbols exported using this macro are put into a module namespace. This
+namespace cannot be imported.
+
+The macro takes a comma separated list of module names, allowing only those
+modules to access this symbol. Simple tail-globs are supported.
+
+For example:
+
+  EXPORT_SYMBOL_GPL_FOR_MODULES(preempt_notifier_inc, "kvm,kvm-*")
+
+will limit usage of this symbol to modules whoes name matches the given
+patterns.
+
 3. How to use Symbols exported in Namespaces
 ============================================
 
@@ -154,3 +173,6 @@ Again, ``make nsdeps`` will eventually a
 You can also run nsdeps for external module builds. A typical usage is::
 
 	$ make -C <path_to_kernel_src> M=$PWD nsdeps
+
+Note: it will happily generate an import statement for the module namespace;
+which will not work and generates build and runtime failures.
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -24,11 +24,17 @@
 	.long sym
 #endif
 
-#define ___EXPORT_SYMBOL(sym, license, ns)		\
+/*
+ * LLVM integrated assembler cam merge adjacent string literals (like
+ * C and GNU-as) passed to '.ascii', but not to '.asciz' and chokes on:
+ *
+ *   .asciz "MODULE_" "kvm" ;
+ */
+#define ___EXPORT_SYMBOL(sym, license, ns...)		\
 	.section ".export_symbol","a"		ASM_NL	\
 	__export_symbol_##sym:			ASM_NL	\
 		.asciz license			ASM_NL	\
-		.asciz ns			ASM_NL	\
+		.ascii ns "\0"			ASM_NL	\
 		__EXPORT_SYMBOL_REF(sym)	ASM_NL	\
 	.previous
 
@@ -85,4 +91,6 @@
 #define EXPORT_SYMBOL_NS(sym, ns)	__EXPORT_SYMBOL(sym, "", ns)
 #define EXPORT_SYMBOL_NS_GPL(sym, ns)	__EXPORT_SYMBOL(sym, "GPL", ns)
 
+#define EXPORT_SYMBOL_GPL_FOR_MODULES(sym, mods) __EXPORT_SYMBOL(sym, "GPL", "module:" mods)
+
 #endif /* _LINUX_EXPORT_H */

Re: [PATCH v3 0/5] module: Strict per-modname namespaces

[Greg KH]

On Fri, May 02, 2025 at 04:12:04PM +0200, Peter Zijlstra wrote:
> Hi!
> 
> Implement means for exports to be available to an explicit list of named
> modules. By explicitly limiting the usage of certain exports, the abuse
> potential/risk is greatly reduced.
> 
> Changes since v2:
> 
>  - switch to "module:" prefix (Masahiro)
>  - removed some patch noise (Masahiro)
>  - strstarts() and strlen() usage for prefixes (Masahiro)
>  - simpler ___EXPORT_SYMBOL() changes (Masahiro)
> 
> Not making using of glob_match() / fnmatch(); this would result in more
> complicated code for very little gain.
> 

Nice work!

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH v3 2/5] module: Add module specific symbol namespace support

[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);

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

Re: [PATCH v3 1/5] modpost: Use for() loop

[Masahiro Yamada]

On Fri, May 2, 2025 at 11:25 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> Slight cleanup by using a for() loop instead of while(). This makes it
> clearer what is the iteration and what is the actual work done.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  scripts/mod/modpost.c |    8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)

Applied to linux-kbuild.
Thanks.



> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1595,12 +1595,10 @@ static void read_symbols(const char *mod
>                         license = get_next_modinfo(&info, "license", license);
>                 }
>
> -               namespace = get_modinfo(&info, "import_ns");
> -               while (namespace) {
> +               for (namespace = get_modinfo(&info, "import_ns");
> +                    namespace;
> +                    namespace = get_next_modinfo(&info, "import_ns", namespace))
>                         add_namespace(&mod->imported_namespaces, namespace);
> -                       namespace = get_next_modinfo(&info, "import_ns",
> -                                                    namespace);
> -               }
>
>                 if (!get_modinfo(&info, "description"))
>                         warn("missing MODULE_DESCRIPTION() in %s\n", modname);
>
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v3 2/5] module: Add module specific symbol namespace support

[Petr Pavlu]

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

Re: [PATCH v3 3/5] module: Extend the MODULE_ namespace parsing

[Petr Pavlu]

On 5/2/25 16:12, Peter Zijlstra 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>

Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>

-- Petr

Re: [PATCH v3 4/5] module: Account for the build time module name mangling

[Petr Pavlu]

On 5/2/25 16:12, Peter Zijlstra wrote:
> Sean noted that scripts/Makefile.lib:name-fix-token rule will mangle
> the module name with s/-/_/g.
> 
> Since this happens late in the build, only the kernel needs to bother
> with this, the modpost tool still sees the original name.
> 
> Reported-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Tested-by: Sean Christopherson <seanjc@google.com>
> ---
>  kernel/module/main.c |   26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -170,6 +170,30 @@ static inline void add_taint_module(stru
>  }
>  
>  /*
> + * Like strncmp(), except s/-/_/g as per scripts/Makefile.lib:name-fix-token rule.
> + */
> +static int mod_strncmp(const char *str_a, const char *str_b, size_t n)
> +{
> +	for (int i = 0; i < n; i++) {

Nit: This could be 'size_t i' for consistency. I can adjust it when
picking up the series.

Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>

-- Petr

> +		char a = str_a[i];
> +		char b = str_b[i];
> +		int d;
> +
> +		if (a == '-') a = '_';
> +		if (b == '-') b = '_';
> +
> +		d = a - b;
> +		if (d)
> +			return d;
> +
> +		if (!a)
> +			break;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> [...]

Re: [PATCH v3 5/5] module: Provide EXPORT_SYMBOL_GPL_FOR_MODULES() helper

[Petr Pavlu]

On 5/2/25 16:12, Peter Zijlstra wrote:
> Helper macro to more easily limit the export of a symbol to a given
> list of modules.
> 
> Eg:
> 
>   EXPORT_SYMBOL_GPL_FOR_MODULES(preempt_notifier_inc, "kvm");
> 
> will limit the use of said function to kvm.ko, any other module trying
> to use this symbol will refure to load (and get modpost build
> failures).
> 
> Requested-by: Masahiro Yamada <masahiroy@kernel.org>
> Requested-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> [...]
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -24,11 +24,17 @@
>  	.long sym
>  #endif
>  
> -#define ___EXPORT_SYMBOL(sym, license, ns)		\
> +/*
> + * LLVM integrated assembler cam merge adjacent string literals (like
> + * C and GNU-as) passed to '.ascii', but not to '.asciz' and chokes on:
> + *
> + *   .asciz "MODULE_" "kvm" ;
> + */

Typo: "cam" -> "can't". I can correct it when picking up the series.

Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>

-- Petr

> +#define ___EXPORT_SYMBOL(sym, license, ns...)		\
>  	.section ".export_symbol","a"		ASM_NL	\
>  	__export_symbol_##sym:			ASM_NL	\
>  		.asciz license			ASM_NL	\
> -		.asciz ns			ASM_NL	\
> +		.ascii ns "\0"			ASM_NL	\
>  		__EXPORT_SYMBOL_REF(sym)	ASM_NL	\
>  	.previous
>  
> @@ -85,4 +91,6 @@
>  #define EXPORT_SYMBOL_NS(sym, ns)	__EXPORT_SYMBOL(sym, "", ns)
>  #define EXPORT_SYMBOL_NS_GPL(sym, ns)	__EXPORT_SYMBOL(sym, "GPL", ns)
>  
> +#define EXPORT_SYMBOL_GPL_FOR_MODULES(sym, mods) __EXPORT_SYMBOL(sym, "GPL", "module:" mods)
> +
>  #endif /* _LINUX_EXPORT_H */
> 
> 

Re: [PATCH v3 0/5] module: Strict per-modname namespaces

[Petr Pavlu]

On 5/2/25 16:12, Peter Zijlstra wrote:
> Hi!
> 
> Implement means for exports to be available to an explicit list of named
> modules. By explicitly limiting the usage of certain exports, the abuse
> potential/risk is greatly reduced.
> 
> Changes since v2:
> 
>  - switch to "module:" prefix (Masahiro)
>  - removed some patch noise (Masahiro)
>  - strstarts() and strlen() usage for prefixes (Masahiro)
>  - simpler ___EXPORT_SYMBOL() changes (Masahiro)
> 
> Not making using of glob_match() / fnmatch(); this would result in more
> complicated code for very little gain.

@Masahiro, please let me know if you're still reviewing the modpost or
other changes, or the series now looks good to you. I'd like to take it
for v6.16-rc1.

-- 
Thanks,
Petr

Re: [PATCH v3 0/5] module: Strict per-modname namespaces

[Masahiro Yamada]

On Wed, May 14, 2025 at 5:48 PM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> On 5/2/25 16:12, Peter Zijlstra wrote:
> > Hi!
> >
> > Implement means for exports to be available to an explicit list of named
> > modules. By explicitly limiting the usage of certain exports, the abuse
> > potential/risk is greatly reduced.
> >
> > Changes since v2:
> >
> >  - switch to "module:" prefix (Masahiro)
> >  - removed some patch noise (Masahiro)
> >  - strstarts() and strlen() usage for prefixes (Masahiro)
> >  - simpler ___EXPORT_SYMBOL() changes (Masahiro)
> >
> > Not making using of glob_match() / fnmatch(); this would result in more
> > complicated code for very little gain.
>
> @Masahiro, please let me know if you're still reviewing the modpost or
> other changes, or the series now looks good to you. I'd like to take it
> for v6.16-rc1.


The first patch was applied to linux-kbuild.

I think I can take it.

Peter did not use the common API for glob matching.
I will check this part.




-- 
Best Regards
Masahiro Yamada

Re: [PATCH v3 2/5] module: Add module specific symbol namespace support

[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

Re: [PATCH v3 2/5] module: Add module specific symbol namespace support

[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>
> ---


>  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

Re: [PATCH v3 0/5] module: Strict per-modname namespaces

[Petr Pavlu]

On 5/17/25 08:48, Masahiro Yamada wrote:
> On Wed, May 14, 2025 at 5:48 PM Petr Pavlu <petr.pavlu@suse.com> wrote:
>>
>> On 5/2/25 16:12, Peter Zijlstra wrote:
>>> Hi!
>>>
>>> Implement means for exports to be available to an explicit list of named
>>> modules. By explicitly limiting the usage of certain exports, the abuse
>>> potential/risk is greatly reduced.
>>>
>>> Changes since v2:
>>>
>>>  - switch to "module:" prefix (Masahiro)
>>>  - removed some patch noise (Masahiro)
>>>  - strstarts() and strlen() usage for prefixes (Masahiro)
>>>  - simpler ___EXPORT_SYMBOL() changes (Masahiro)
>>>
>>> Not making using of glob_match() / fnmatch(); this would result in more
>>> complicated code for very little gain.
>>
>> @Masahiro, please let me know if you're still reviewing the modpost or
>> other changes, or the series now looks good to you. I'd like to take it
>> for v6.16-rc1.
> 
> 
> The first patch was applied to linux-kbuild.
> 
> I think I can take it.

Ok, that works for me.

-- 
Thanks,
Petr

Re: [PATCH v3 3/5] module: Extend the MODULE_ namespace parsing

[Masahiro Yamada]

I think the patch subject is stale.

MODULE_ was the prefix in the previous v2 series.

Now, the prefix part is module:


On Fri, May 2, 2025 at 11:25 PM 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  |   36 ++++++++++++++++++++++++++++++++++--
>  scripts/mod/modpost.c |   36 ++++++++++++++++++++++++++++++++++--
>  2 files changed, 68 insertions(+), 4 deletions(-)
>
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -1083,12 +1083,44 @@ static char *get_modinfo(const struct lo
>         return get_next_modinfo(info, tag, NULL);
>  }
>
> +/**
> + * verify_module_namespace() - does @modname have access to this symbol's @namespace
> + * @namespace: export symbol namespace
> + * @modname: module name
> + *
> + * If @namespace is prefixed with "module:" to indicate it is a module namespace
> + * then test if @modname matches any of the comma separated patterns.
> + *
> + * The patterns only support tail-glob.
> + */
>  static bool verify_module_namespace(const char *namespace, const char *modname)
>  {
> +       size_t len, modlen = strlen(modname);
>         const char *prefix = "module:";
> +       const char *sep;
> +       bool glob;
>
> -       return strstarts(namespace, prefix) &&
> -              !strsmp(namespace + strlen(prefix), modname);
> +       if (!strstarts(namespace, prefix))
> +               return false;
> +
> +       for (namespace += strlen(prefix); *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,
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1682,12 +1682,44 @@ void buf_write(struct buffer *buf, const
>         buf->pos += len;
>  }
>
> +/**
> + * verify_module_namespace() - does @modname have access to this symbol's @namespace
> + * @namespace: export symbol namespace
> + * @modname: module name
> + *
> + * If @namespace is prefixed with "module:" to indicate it is a module namespace
> + * then test if @modname matches any of the comma separated patterns.
> + *
> + * The patterns only support tail-glob.
> + */
>  static bool verify_module_namespace(const char *namespace, const char *modname)
>  {
> +       size_t len, modlen = strlen(modname);
>         const char *prefix = "module:";
> +       const char *sep;
> +       bool glob;
>
> -       return strstarts(namespace, prefix) &&
> -              !strcmp(namespace + strlen(prefix), modname);
> +       if (!strstarts(namespace, prefix))
> +               return false;
> +
> +       for (namespace += strlen(prefix); *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)
>
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v3 2/5] module: Add module specific symbol namespace support

[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

Re: [PATCH v3 3/5] module: Extend the MODULE_ namespace parsing

[Masahiro Yamada]

On Wed, May 21, 2025 at 9:08 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> I think the patch subject is stale.
>
> MODULE_ was the prefix in the previous v2 series.
>
> Now, the prefix part is module:


I will apply with the subject fixed, as "MODULE_" prefix does not make
sense any more.


"module: Extend the module namespace parsing"




>
>
> On Fri, May 2, 2025 at 11:25 PM 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  |   36 ++++++++++++++++++++++++++++++++++--
> >  scripts/mod/modpost.c |   36 ++++++++++++++++++++++++++++++++++--
> >  2 files changed, 68 insertions(+), 4 deletions(-)
> >
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> > @@ -1083,12 +1083,44 @@ static char *get_modinfo(const struct lo
> >         return get_next_modinfo(info, tag, NULL);
> >  }
> >
> > +/**
> > + * verify_module_namespace() - does @modname have access to this symbol's @namespace
> > + * @namespace: export symbol namespace
> > + * @modname: module name
> > + *
> > + * If @namespace is prefixed with "module:" to indicate it is a module namespace
> > + * then test if @modname matches any of the comma separated patterns.
> > + *
> > + * The patterns only support tail-glob.
> > + */
> >  static bool verify_module_namespace(const char *namespace, const char *modname)
> >  {
> > +       size_t len, modlen = strlen(modname);
> >         const char *prefix = "module:";
> > +       const char *sep;
> > +       bool glob;
> >
> > -       return strstarts(namespace, prefix) &&
> > -              !strsmp(namespace + strlen(prefix), modname);
> > +       if (!strstarts(namespace, prefix))
> > +               return false;
> > +
> > +       for (namespace += strlen(prefix); *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,
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -1682,12 +1682,44 @@ void buf_write(struct buffer *buf, const
> >         buf->pos += len;
> >  }
> >
> > +/**
> > + * verify_module_namespace() - does @modname have access to this symbol's @namespace
> > + * @namespace: export symbol namespace
> > + * @modname: module name
> > + *
> > + * If @namespace is prefixed with "module:" to indicate it is a module namespace
> > + * then test if @modname matches any of the comma separated patterns.
> > + *
> > + * The patterns only support tail-glob.
> > + */
> >  static bool verify_module_namespace(const char *namespace, const char *modname)
> >  {
> > +       size_t len, modlen = strlen(modname);
> >         const char *prefix = "module:";
> > +       const char *sep;
> > +       bool glob;
> >
> > -       return strstarts(namespace, prefix) &&
> > -              !strcmp(namespace + strlen(prefix), modname);
> > +       if (!strstarts(namespace, prefix))
> > +               return false;
> > +
> > +       for (namespace += strlen(prefix); *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)
> >
> >
>
>
> --
> Best Regards
> Masahiro Yamada



-- 
Best Regards
Masahiro Yamada