[RFC][PATCH 0/8] module: Strict per-modname namespaces

[RFC][PATCH 0/8] module: Strict per-modname namespaces

[Peter Zijlstra]

Hi!

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

The first three 'patches' clean up the existing export namespace code along the
same lines of 33def8498fdd ("treewide: Convert macro and uses of __section(foo)
to __section("foo")") and for the same reason, it is not desired for the
namespace argument to be a macro expansion itself.

In fact, the second patch is really only a script, because sending the output
to the list is a giant waste of bandwidth. Whoever eventually commits this to a
git tree should squash these first three patches.

The remainder of the patches introduce the special "MODULE_<modname-list>"
namespace, which shall be forbidden from being explicitly imported. A module
that matches the simple modname-list will get an implicit import.

Lightly tested with something like:

git grep -l EXPORT_SYMBOL arch/x86/kvm/ | while read file;
do
  sed -i -e 's/EXPORT_SYMBOL_GPL(\(.[^)]*\))/EXPORT_SYMBOL_GPL_FOR(\1, "kvm,kvm-intel,kvm-amd")/g' $file;
done

Also available at:

  git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git module/namespace

[RFC][PATCH 1/8] module: Prepare for script

[Peter Zijlstra]

Since sed doesn't like multi-line make sure all EXPORT_SYMBOL_NS
things are a single line.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 drivers/crypto/marvell/octeontx2/otx2_cptlf.c |   12 ++++--------
 include/kunit/visibility.h                    |    3 +--
 sound/soc/codecs/tas2781-fmwlib.c             |    6 ++----
 3 files changed, 7 insertions(+), 14 deletions(-)

--- a/drivers/crypto/marvell/octeontx2/otx2_cptlf.c
+++ b/drivers/crypto/marvell/octeontx2/otx2_cptlf.c
@@ -288,8 +288,7 @@ void otx2_cptlf_unregister_misc_interrup
 
 	cptlf_set_misc_intrs(lfs, false);
 }
-EXPORT_SYMBOL_NS_GPL(otx2_cptlf_unregister_misc_interrupts,
-		     CRYPTO_DEV_OCTEONTX2_CPT);
+EXPORT_SYMBOL_NS_GPL(otx2_cptlf_unregister_misc_interrupts, CRYPTO_DEV_OCTEONTX2_CPT);
 
 void otx2_cptlf_unregister_done_interrupts(struct otx2_cptlfs_info *lfs)
 {
@@ -308,8 +307,7 @@ void otx2_cptlf_unregister_done_interrup
 
 	cptlf_set_done_intrs(lfs, false);
 }
-EXPORT_SYMBOL_NS_GPL(otx2_cptlf_unregister_done_interrupts,
-		     CRYPTO_DEV_OCTEONTX2_CPT);
+EXPORT_SYMBOL_NS_GPL(otx2_cptlf_unregister_done_interrupts, CRYPTO_DEV_OCTEONTX2_CPT);
 
 static int cptlf_do_register_interrrupts(struct otx2_cptlfs_info *lfs,
 					 int lf_num, int irq_offset,
@@ -351,8 +349,7 @@ int otx2_cptlf_register_misc_interrupts(
 	otx2_cptlf_unregister_misc_interrupts(lfs);
 	return ret;
 }
-EXPORT_SYMBOL_NS_GPL(otx2_cptlf_register_misc_interrupts,
-		     CRYPTO_DEV_OCTEONTX2_CPT);
+EXPORT_SYMBOL_NS_GPL(otx2_cptlf_register_misc_interrupts, CRYPTO_DEV_OCTEONTX2_CPT);
 
 int otx2_cptlf_register_done_interrupts(struct otx2_cptlfs_info *lfs)
 {
@@ -375,8 +372,7 @@ int otx2_cptlf_register_done_interrupts(
 	otx2_cptlf_unregister_done_interrupts(lfs);
 	return ret;
 }
-EXPORT_SYMBOL_NS_GPL(otx2_cptlf_register_done_interrupts,
-		     CRYPTO_DEV_OCTEONTX2_CPT);
+EXPORT_SYMBOL_NS_GPL(otx2_cptlf_register_done_interrupts, CRYPTO_DEV_OCTEONTX2_CPT);
 
 void otx2_cptlf_free_irqs_affinity(struct otx2_cptlfs_info *lfs)
 {
--- a/include/kunit/visibility.h
+++ b/include/kunit/visibility.h
@@ -24,8 +24,7 @@
      * in test file in order to use symbols.
      * @symbol: the symbol identifier to export
      */
-    #define EXPORT_SYMBOL_IF_KUNIT(symbol) EXPORT_SYMBOL_NS(symbol, \
-	    EXPORTED_FOR_KUNIT_TESTING)
+    #define EXPORT_SYMBOL_IF_KUNIT(symbol) EXPORT_SYMBOL_NS(symbol, EXPORTED_FOR_KUNIT_TESTING)
 #else
     #define VISIBLE_IF_KUNIT static
     #define EXPORT_SYMBOL_IF_KUNIT(symbol)
--- a/sound/soc/codecs/tas2781-fmwlib.c
+++ b/sound/soc/codecs/tas2781-fmwlib.c
@@ -2310,8 +2310,7 @@ int tasdevice_select_tuningprm_cfg(void
 out:
 	return prog_status;
 }
-EXPORT_SYMBOL_NS_GPL(tasdevice_select_tuningprm_cfg,
-	SND_SOC_TAS2781_FMWLIB);
+EXPORT_SYMBOL_NS_GPL(tasdevice_select_tuningprm_cfg, SND_SOC_TAS2781_FMWLIB);
 
 int tasdevice_prmg_load(void *context, int prm_no)
 {
@@ -2392,8 +2391,7 @@ void tasdevice_tuning_switch(void *conte
 			TASDEVICE_BIN_BLK_PRE_SHUTDOWN);
 	}
 }
-EXPORT_SYMBOL_NS_GPL(tasdevice_tuning_switch,
-	SND_SOC_TAS2781_FMWLIB);
+EXPORT_SYMBOL_NS_GPL(tasdevice_tuning_switch, SND_SOC_TAS2781_FMWLIB);
 
 MODULE_DESCRIPTION("Texas Firmware Support");
 MODULE_AUTHOR("Shenghao Ding, TI, <shenghao-ding@ti.com>");

Re: [RFC][PATCH 1/8] module: Prepare for script

[Christoph Hellwig]

On Mon, Nov 11, 2024 at 11:54:31AM +0100, Peter Zijlstra wrote:
> Since sed doesn't like multi-line make sure all EXPORT_SYMBOL_NS
> things are a single line.

Eww.  Just use coccinelle or another tool not so simplistic.

> @@ -375,8 +372,7 @@ int otx2_cptlf_register_done_interrupts(
>  	otx2_cptlf_unregister_done_interrupts(lfs);
>  	return ret;
>  }
> -EXPORT_SYMBOL_NS_GPL(otx2_cptlf_register_done_interrupts,
> -		     CRYPTO_DEV_OCTEONTX2_CPT);
> +EXPORT_SYMBOL_NS_GPL(otx2_cptlf_register_done_interrupts, CRYPTO_DEV_OCTEONTX2_CPT);
>  
>  void otx2_cptlf_free_irqs_affinity(struct otx2_cptlfs_info *lfs)
>  {
> --- a/include/kunit/visibility.h
> +++ b/include/kunit/visibility.h
> @@ -24,8 +24,7 @@
>       * in test file in order to use symbols.
>       * @symbol: the symbol identifier to export
>       */
> -    #define EXPORT_SYMBOL_IF_KUNIT(symbol) EXPORT_SYMBOL_NS(symbol, \
> -	    EXPORTED_FOR_KUNIT_TESTING)
> +    #define EXPORT_SYMBOL_IF_KUNIT(symbol) EXPORT_SYMBOL_NS(symbol, EXPORTED_FOR_KUNIT_TESTING)
>  #else
>      #define VISIBLE_IF_KUNIT static
>      #define EXPORT_SYMBOL_IF_KUNIT(symbol)
> --- a/sound/soc/codecs/tas2781-fmwlib.c
> +++ b/sound/soc/codecs/tas2781-fmwlib.c
> @@ -2310,8 +2310,7 @@ int tasdevice_select_tuningprm_cfg(void
>  out:
>  	return prog_status;
>  }
> -EXPORT_SYMBOL_NS_GPL(tasdevice_select_tuningprm_cfg,
> -	SND_SOC_TAS2781_FMWLIB);
> +EXPORT_SYMBOL_NS_GPL(tasdevice_select_tuningprm_cfg, SND_SOC_TAS2781_FMWLIB);
>  
>  int tasdevice_prmg_load(void *context, int prm_no)
>  {
> @@ -2392,8 +2391,7 @@ void tasdevice_tuning_switch(void *conte
>  			TASDEVICE_BIN_BLK_PRE_SHUTDOWN);
>  	}
>  }
> -EXPORT_SYMBOL_NS_GPL(tasdevice_tuning_switch,
> -	SND_SOC_TAS2781_FMWLIB);
> +EXPORT_SYMBOL_NS_GPL(tasdevice_tuning_switch, SND_SOC_TAS2781_FMWLIB);
>  
>  MODULE_DESCRIPTION("Texas Firmware Support");
>  MODULE_AUTHOR("Shenghao Ding, TI, <shenghao-ding@ti.com>");
> 
> 
---end quoted text---

Re: [RFC][PATCH 1/8] module: Prepare for script

[Peter Zijlstra]

On Mon, Nov 11, 2024 at 03:36:25AM -0800, Christoph Hellwig wrote:
> On Mon, Nov 11, 2024 at 11:54:31AM +0100, Peter Zijlstra wrote:
> > Since sed doesn't like multi-line make sure all EXPORT_SYMBOL_NS
> > things are a single line.
> 
> Eww.  Just use coccinelle or another tool not so simplistic.

Feel free to do so. I've never managed to get coccinelle to do anything.

Re: [RFC][PATCH 1/8] module: Prepare for script

[Peter Zijlstra]

On Mon, Nov 11, 2024 at 01:55:29PM +0100, Peter Zijlstra wrote:
> On Mon, Nov 11, 2024 at 03:36:25AM -0800, Christoph Hellwig wrote:
> > On Mon, Nov 11, 2024 at 11:54:31AM +0100, Peter Zijlstra wrote:
> > > Since sed doesn't like multi-line make sure all EXPORT_SYMBOL_NS
> > > things are a single line.
> > 
> > Eww.  Just use coccinelle or another tool not so simplistic.
> 
> Feel free to do so. I've never managed to get coccinelle to do anything.

So I put a little more effort in and got you this (awk needs to be
gawk>=4.1)

git grep -l -e MODULE_IMPORT_NS -e EXPORT_SYMBOL_NS | while read file;
do
  awk -i inplace '
    /^#define EXPORT_SYMBOL_NS/ {
      gsub(/__stringify\(ns\)/, "ns");
      print;
      next;
    }
    /^#define MODULE_IMPORT_NS/ {
      gsub(/__stringify\(ns\)/, "ns");
      print;
      next;
    }
    /MODULE_IMPORT_NS/ {
      $0 = gensub(/MODULE_IMPORT_NS\(([^)]*)\)/, "MODULE_IMPORT_NS(\"\\1\")", "g");
    }
    /EXPORT_SYMBOL_NS/ {
      if ($0 ~ /(EXPORT_SYMBOL_NS[^(]*)\(([^,]+),/) {
        if ($0 !~ /(EXPORT_SYMBOL_NS[^(]*)\(([^,]+), ([^)]+)\)/ &&
            $0 !~ /(EXPORT_SYMBOL_NS[^(]*)\(\)/ &&
            $0 !~ /^my/) {
          getline line;
          gsub(/[[:space:]]*\\$/, "");
          gsub(/[[:space:]]/, "", line);
          $0 = $0 " " line;
        }

        $0 = gensub(/(EXPORT_SYMBOL_NS[^(]*)\(([^,]+), ([^)]+)\)/,
                    "\\1(\\2, \"\\3\")", "g");
      }
    }
    { print }' $file;
done


I'm sure that wasn't worth the time I spend on it though :/

[RFC][PATCH 2/8] module: Convert symbol namespace to string literal

[Peter Zijlstra]

git grep -l -e MODULE_IMPORT_NS -e EXPORT_SYMBOL_NS | while read file;
do
  sed -i -e 's/MODULE_IMPORT_NS(\([^)]*\))/MODULE_IMPORT_NS("\1")/g' \
         -e 's/\(EXPORT_SYMBOL_NS[^(]*\)(\([^,]*\), \([^)]*\))/\1(\2, "\3")/g' $file;
done

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---

[RFC][PATCH 3/8] module: Fix up after script

[Peter Zijlstra]

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/export.h |    4 ++--
 include/linux/module.h |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -67,7 +67,7 @@
 
 #define EXPORT_SYMBOL(sym)		_EXPORT_SYMBOL(sym, "")
 #define EXPORT_SYMBOL_GPL(sym)		_EXPORT_SYMBOL(sym, "GPL")
-#define EXPORT_SYMBOL_NS(sym, "ns")	__EXPORT_SYMBOL(sym, "", __stringify(ns))
-#define EXPORT_SYMBOL_NS_GPL(sym, "ns")	__EXPORT_SYMBOL(sym, "GPL", __stringify(ns))
+#define EXPORT_SYMBOL_NS(sym, ns)	__EXPORT_SYMBOL(sym, "", ns)
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)	__EXPORT_SYMBOL(sym, "GPL", ns)
 
 #endif /* _LINUX_EXPORT_H */
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -296,7 +296,7 @@ extern typeof(name) __mod_##type##__##na
  * files require multiple MODULE_FIRMWARE() specifiers */
 #define MODULE_FIRMWARE(_firmware) MODULE_INFO(firmware, _firmware)
 
-#define MODULE_IMPORT_NS("ns")	MODULE_INFO(import_ns, __stringify(ns))
+#define MODULE_IMPORT_NS(ns)	MODULE_INFO(import_ns, ns)
 
 struct notifier_block;
 

[RFC][PATCH 4/8] module/modpost: Use for() loop

[Peter Zijlstra]

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

--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1586,12 +1586,9 @@ 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 (extra_warn && !get_modinfo(&info, "description"))
 			warn("missing MODULE_DESCRIPTION() in %s\n", modname);

[RFC][PATCH 5/8] module/modpost: Add basename helper

[Peter Zijlstra]

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

--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1553,6 +1553,16 @@ static void mod_set_crcs(struct module *
 	free(buf);
 }
 
+static const char *mod_basename(const char *modname)
+{
+	const char *basename = strrchr(modname, '/');
+	if (basename)
+		basename++;
+	else
+		basename = modname;
+	return basename;
+}
+
 static void read_symbols(const char *modname)
 {
 	const char *symname;
@@ -1697,11 +1707,7 @@ static void check_exports(struct module
 		s->crc_valid = exp->crc_valid;
 		s->crc = exp->crc;
 
-		basename = strrchr(mod->name, '/');
-		if (basename)
-			basename++;
-		else
-			basename = mod->name;
+		basename = mod_basename(mod->name);
 
 		if (!contains_namespace(&mod->imported_namespaces, exp->namespace)) {
 			modpost_log(!allow_missing_ns_imports,

[RFC][PATCH 6/8] 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. Modules implicitly get the "MODULE_${modname}" namespace
added.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/module/main.c  |   28 ++++++++++++++++++++++++++--
 scripts/mod/modpost.c |    5 +++++
 2 files changed, 31 insertions(+), 2 deletions(-)

--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1070,6 +1070,13 @@ 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)
+			return 0;
+
 		for_each_modinfo_entry(imported_namespace, info, "import_ns") {
 			if (strcmp(namespace, imported_namespace) == 0)
 				return 0;
@@ -1613,15 +1620,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)
 {
 	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)
@@ -2935,7 +2957,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
@@ -1565,6 +1565,7 @@ 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;
@@ -1600,6 +1601,10 @@ 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);
 	}

Re: [RFC][PATCH 6/8] module: Add module specific symbol namespace support

[Christoph Hellwig]

On Mon, Nov 11, 2024 at 11:54:36AM +0100, 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. Modules implicitly get the "MODULE_${modname}" namespace
> added.

Btw, I finally remember why I wanted a separate macro for this:
so that we can also add the config symbol as an argument and not
export the symbol if the module isn't configured or built in.

Re: [RFC][PATCH 6/8] module: Add module specific symbol namespace support

[Sean Christopherson]

On Mon, Nov 11, 2024, Christoph Hellwig wrote:
> On Mon, Nov 11, 2024 at 11:54:36AM +0100, 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. Modules implicitly get the "MODULE_${modname}" namespace
> > added.
> 
> Btw, I finally remember why I wanted a separate macro for this:
> so that we can also add the config symbol as an argument and not
> export the symbol if the module isn't configured or built in.

That could get ugly, especially in generic code, as multiple KVM architectures
use multiple modules, e.g. x86 generates kvm.ko, and then vendor specific modules
kvm-amd.ko and kvm-intel.ko; and PPC generates kvm.ko, and kvm-hv.ko and kvm-pr.ko.
PPC in particular is annoying because it generates kvm.ko for KVM_BOOK3S_32=m or
KVM_BOOK3S_64=m.

The other quirk is that, on x86 at least, kvm.ko is now built if and only if at
least one of KVM_AMD=m or KVM_INTEL=m, which triggers KVM_X86=m.  I.e. kvm.ko isn't
built if there are no vendor modules, even if KVM=m.

I'd also like to use this infrastructure to restrict KVM's own exports, e.g. so
that KVM exports its symbols for kvm-{amd,intel,hv,pr}.ko only as needed.

So rather than having EXPORT_SYMBOL_GPL_FOR() deal with KVM's messes, would it
instead make sense to have KVM provide EXPORT_SYMBOL_GPL_FOR_KVM()?  Then KVM can
reuse the painful extrapolation of Kconfigs to module names for its own exports.
And IMO, that'd make the code that does the exports much more readable, too.

E.g. for x86, something like:

#if IS_MODULE(CONFIG_KVM_AMD) && IS_MODULE(CONFIG_KVM_INTEL)
#define KVM_VENDOR_MODULES kvm-amd,kvm-intel
#elif IS_MODULE(CONFIG_KVM_AMD)
#define KVM_VENDOR_MODULES kvm-amd
#elif IS_MODULE(CONFIG_KVM_INTEL)
#define KVM_VENDOR_MODULES kvm-intel
#else
#undef KVM_VENDOR_MODULES
#endif

#ifdef KVM_VENDOR_MODULES
static_assert(IS_MODULE(CONFIG_KVM_X86));

#define EXPORT_SYMBOL_GPL_FOR_KVM_INTERNAL(symbol) \
	EXPORT_SYMBOL_GPL_FOR(symbol, __stringify(KVM_VENDOR_MODULES))
#define EXPORT_SYMBOL_GPL_FOR_KVM(symbol) \
	EXPORT_SYMBOL_GPL_FOR(symbol, "kvm," __stringify(KVM_VENDOR_MODULES))
#else
EXPORT_SYMBOL_GPL_FOR_KVM_INTERNAL(symbol)
EXPORT_SYMBOL_GPL_FOR_KVM(symbol)
#endif

Re: [RFC][PATCH 6/8] module: Add module specific symbol namespace support

[Peter Zijlstra]

On Mon, Nov 11, 2024 at 10:36:16AM -0800, Sean Christopherson wrote:
> E.g. for x86, something like:
> 
> #if IS_MODULE(CONFIG_KVM_AMD) && IS_MODULE(CONFIG_KVM_INTEL)
> #define KVM_VENDOR_MODULES kvm-amd,kvm-intel
> #elif IS_MODULE(CONFIG_KVM_AMD)
> #define KVM_VENDOR_MODULES kvm-amd
> #elif IS_MODULE(CONFIG_KVM_INTEL)
> #define KVM_VENDOR_MODULES kvm-intel
> #else
> #undef KVM_VENDOR_MODULES
> #endif
> 
> #ifdef KVM_VENDOR_MODULES
> static_assert(IS_MODULE(CONFIG_KVM_X86));
> 
> #define EXPORT_SYMBOL_GPL_FOR_KVM_INTERNAL(symbol) \
> 	EXPORT_SYMBOL_GPL_FOR(symbol, __stringify(KVM_VENDOR_MODULES))
> #define EXPORT_SYMBOL_GPL_FOR_KVM(symbol) \
> 	EXPORT_SYMBOL_GPL_FOR(symbol, "kvm," __stringify(KVM_VENDOR_MODULES))
> #else
> EXPORT_SYMBOL_GPL_FOR_KVM_INTERNAL(symbol)
> EXPORT_SYMBOL_GPL_FOR_KVM(symbol)
> #endif

I see no problem with KVM doing something like this on top of whatever
we end up with.

[RFC][PATCH 7/8] 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  |   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'
+ */
+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);

[RFC][PATCH 8/8] module: Provide EXPORT_SYMBOL*_FOR() helpers

[Peter Zijlstra]

Because Christoph and Masahiro-san asked for it :-)

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/export.h |    3 +++
 1 file changed, 3 insertions(+)

--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -70,4 +70,7 @@
 #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_FOR(sym, mods)	EXPORT_SYMBOL_NS(sym, "MODULE_" mods)
+#define EXPORT_SYMBOL_GPL_FOR(sym, mods) EXPORT_SYMBOL_NS_GPL(sym, "MODULE_" mods)
+
 #endif /* _LINUX_EXPORT_H */

Re: [RFC][PATCH 8/8] module: Provide EXPORT_SYMBOL*_FOR() helpers

[Christoph Hellwig]

On Mon, Nov 11, 2024 at 11:54:38AM +0100, Peter Zijlstra wrote:
> +#define EXPORT_SYMBOL_FOR(sym, mods)	EXPORT_SYMBOL_NS(sym, "MODULE_" mods)

I don't think we need a non-_GPL version of this, as the exports
obviously are for in-tree modules only.

Re: [RFC][PATCH 0/8] module: Strict per-modname namespaces

[Sean Christopherson]

On Mon, Nov 11, 2024, Peter Zijlstra wrote:
> Hi!
> 
> Implement a means for exports to be available only to an explicit list of named
> modules. By explicitly limiting the usage of certain exports, the abuse
> potential/risk is greatly reduced.
> 
> The first three 'patches' clean up the existing export namespace code along the
> same lines of 33def8498fdd ("treewide: Convert macro and uses of __section(foo)
> to __section("foo")") and for the same reason, it is not desired for the
> namespace argument to be a macro expansion itself.
> 
> In fact, the second patch is really only a script, because sending the output
> to the list is a giant waste of bandwidth. Whoever eventually commits this to a
> git tree should squash these first three patches.
> 
> The remainder of the patches introduce the special "MODULE_<modname-list>"
> namespace, which shall be forbidden from being explicitly imported. A module
> that matches the simple modname-list will get an implicit import.
> 
> Lightly tested with something like:
> 
> git grep -l EXPORT_SYMBOL arch/x86/kvm/ | while read file;
> do
>   sed -i -e 's/EXPORT_SYMBOL_GPL(\(.[^)]*\))/EXPORT_SYMBOL_GPL_FOR(\1, "kvm,kvm-intel,kvm-amd")/g' $file;
> done

Heh, darn modules.  This will compile just fine, but if the module contains a
dash, loading the module will fail because scripts/Makefile.lib replaces the dash
with an underscore the build name.  E.g. "kvm-intel" at compile time generates
kvm-intel.ko, but the actual name of the module as seen by the kernel is kvm_intel.

--
# These flags are needed for modversions and compiling, so we define them here
# $(modname_flags) defines KBUILD_MODNAME as the name of the module it will
# end up in (or would, if it gets compiled in)
name-fix-token = $(subst $(comma),_,$(subst -,_,$1))  <====================
name-fix = $(call stringify,$(call name-fix-token,$1))
basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget))
modname_flags  = -DKBUILD_MODNAME=$(call name-fix,$(modname)) \
		 -D__KBUILD_MODNAME=kmod_$(call name-fix-token,$(modname))
modfile_flags  = -DKBUILD_MODFILE=$(call stringify,$(modfile))
--

Re: [RFC][PATCH 0/8] module: Strict per-modname namespaces

[Peter Zijlstra]

On Mon, Nov 11, 2024 at 04:48:58PM -0800, Sean Christopherson wrote:
> On Mon, Nov 11, 2024, Peter Zijlstra wrote:
> > Hi!
> > 
> > Implement a means for exports to be available only to an explicit list of named
> > modules. By explicitly limiting the usage of certain exports, the abuse
> > potential/risk is greatly reduced.
> > 
> > The first three 'patches' clean up the existing export namespace code along the
> > same lines of 33def8498fdd ("treewide: Convert macro and uses of __section(foo)
> > to __section("foo")") and for the same reason, it is not desired for the
> > namespace argument to be a macro expansion itself.
> > 
> > In fact, the second patch is really only a script, because sending the output
> > to the list is a giant waste of bandwidth. Whoever eventually commits this to a
> > git tree should squash these first three patches.
> > 
> > The remainder of the patches introduce the special "MODULE_<modname-list>"
> > namespace, which shall be forbidden from being explicitly imported. A module
> > that matches the simple modname-list will get an implicit import.
> > 
> > Lightly tested with something like:
> > 
> > git grep -l EXPORT_SYMBOL arch/x86/kvm/ | while read file;
> > do
> >   sed -i -e 's/EXPORT_SYMBOL_GPL(\(.[^)]*\))/EXPORT_SYMBOL_GPL_FOR(\1, "kvm,kvm-intel,kvm-amd")/g' $file;
> > done
> 
> Heh, darn modules.  This will compile just fine, but if the module contains a
> dash, loading the module will fail because scripts/Makefile.lib replaces the dash
> with an underscore the build name.  E.g. "kvm-intel" at compile time generates
> kvm-intel.ko, but the actual name of the module as seen by the kernel is kvm_intel.

I was wondering about that...  WTH is kvm doing that? I mean, I suppose
you can do: "kvm-intel,kvm_intel" but that's somewhat tedious.

Re: [RFC][PATCH 0/8] module: Strict per-modname namespaces

[Sean Christopherson]

On Tue, Nov 12, 2024, Peter Zijlstra wrote:
> On Mon, Nov 11, 2024 at 04:48:58PM -0800, Sean Christopherson wrote:
> > On Mon, Nov 11, 2024, Peter Zijlstra wrote:
> > > Hi!
> > > 
> > > Implement a means for exports to be available only to an explicit list of named
> > > modules. By explicitly limiting the usage of certain exports, the abuse
> > > potential/risk is greatly reduced.
> > > 
> > > The first three 'patches' clean up the existing export namespace code along the
> > > same lines of 33def8498fdd ("treewide: Convert macro and uses of __section(foo)
> > > to __section("foo")") and for the same reason, it is not desired for the
> > > namespace argument to be a macro expansion itself.
> > > 
> > > In fact, the second patch is really only a script, because sending the output
> > > to the list is a giant waste of bandwidth. Whoever eventually commits this to a
> > > git tree should squash these first three patches.
> > > 
> > > The remainder of the patches introduce the special "MODULE_<modname-list>"
> > > namespace, which shall be forbidden from being explicitly imported. A module
> > > that matches the simple modname-list will get an implicit import.
> > > 
> > > Lightly tested with something like:
> > > 
> > > git grep -l EXPORT_SYMBOL arch/x86/kvm/ | while read file;
> > > do
> > >   sed -i -e 's/EXPORT_SYMBOL_GPL(\(.[^)]*\))/EXPORT_SYMBOL_GPL_FOR(\1, "kvm,kvm-intel,kvm-amd")/g' $file;
> > > done
> > 
> > Heh, darn modules.  This will compile just fine, but if the module contains a
> > dash, loading the module will fail because scripts/Makefile.lib replaces the dash
> > with an underscore the build name.  E.g. "kvm-intel" at compile time generates
> > kvm-intel.ko, but the actual name of the module as seen by the kernel is kvm_intel.
> 
> I was wondering about that...  WTH is kvm doing that?

No idea.  The naming has been that way since KVM's inception in commit 6aa8b732ca01
("[PATCH] kvm: userspace interface").  My guess is that either no one noticed, or
those who noticed didn't care.

FWIW, IMO the kernel build system is the one that's being weird.  AFAICT, the
'-' => '_' conversion was added so that spinlocks could be placed into unique
subsections.  Amusingly, it doesn't appear that there are any remaining users of
LOCK_SECTION_NAME.

  commit b5635319d32438ed516568f53013a460ba16e6ee
  Author:     Dave Jones <davej@suse.de>
  AuthorDate: Fri Feb 8 01:43:23 2002 -0800
  Commit:     Linus Torvalds <torvalds@penguin.transmeta.com>
  CommitDate: Fri Feb 8 01:43:23 2002 -0800

    [PATCH] text.lock -> subsection changes.
    
    Make spinlocks etc use subsections of their parent sections instead of
    an ELF section of their own - needed for newer binutils when the parent
    sector is removed.

#define LOCK_SECTION_NAME ".text..lock."KBUILD_BASENAME

#define LOCK_SECTION_START(extra)               \
        ".subsection 1\n\t"                     \
        extra                                   \
        ".ifndef " LOCK_SECTION_NAME "\n\t"     \
        LOCK_SECTION_NAME ":\n\t"               \
        ".endif\n"

#define LOCK_SECTION_END                        \
        ".previous\n\t"

#define __lockfunc __section(".spinlock.text")


> I mean, I suppose you can do: "kvm-intel,kvm_intel" but that's somewhat
> tedious.

This likely needs to be addressed in whatever chunk of code is enforcing the
namespaces.  The s/-/_ behavior (and vice versa!) is *very* baked into the kernel
at this point, e.g. parameqn() will happily parse dashes or underscores for every
kernel parameter.  As horrific as it is, I think the module namespace needs to do
the same, i.e. treat dashes and underscores as one and the same.


More historical amusement:

commit 8863179c65618844379ef90d4a708293042465c8
Author:     Andrew Morton <akpm@digeo.com>
AuthorDate: Sun Feb 2 06:08:27 2003 -0800
Commit:     Linus Torvalds <torvalds@home.transmeta.com>
CommitDate: Sun Feb 2 06:08:27 2003 -0800

    [PATCH] kernel param and KBUILD_MODNAME name-munging mess
    
    Patch from: Rusty Russell <rusty@rustcorp.com.au>
    
    Mikael Pettersson points out that "-s" gets mangled to "_s" on the
    kernel command line, even though it turns out not to be a
    parameter.

commit 326e7842d30d5cfc1089b85a7aa63e5c9f3c0a74
Author:     Rusty Russell <rusty@rustcorp.com.au>
AuthorDate: Sat Dec 14 20:13:11 2002 -0800
Commit:     Linus Torvalds <torvalds@home.transmeta.com>
CommitDate: Sat Dec 14 20:13:11 2002 -0800

    [PATCH] Module Parameter Core Patch
    
    This patch is a rewrite of the insmod and boot parameter handling,
    to unify them.
    
    The new format is fairly simple: built on top of __module_param_call there
    are several helpers, eg "module_param(foo, int, 000)".  The final argument
    is the permissions bits, for exposing parameters in sysfs (if
    non-zero) at a later stage.

Re: [RFC][PATCH 0/8] module: Strict per-modname namespaces

[Peter Zijlstra]

On Tue, Nov 12, 2024 at 09:56:20AM -0800, Sean Christopherson wrote:

> This likely needs to be addressed in whatever chunk of code is enforcing the
> namespaces.  The s/-/_ behavior (and vice versa!) is *very* baked into the kernel
> at this point, e.g. parameqn() will happily parse dashes or underscores for every
> kernel parameter.  As horrific as it is, I think the module namespace needs to do
> the same, i.e. treat dashes and underscores as one and the same.

Right, I'll add a s/-/_/g on both ends of the strcmp or somesuch.

Re: [RFC][PATCH 0/8] module: Strict per-modname namespaces

[Peter Zijlstra]

On Tue, Nov 12, 2024 at 08:52:26PM +0100, Peter Zijlstra wrote:
> On Tue, Nov 12, 2024 at 09:56:20AM -0800, Sean Christopherson wrote:
> 
> > This likely needs to be addressed in whatever chunk of code is enforcing the
> > namespaces.  The s/-/_ behavior (and vice versa!) is *very* baked into the kernel
> > at this point, e.g. parameqn() will happily parse dashes or underscores for every
> > kernel parameter.  As horrific as it is, I think the module namespace needs to do
> > the same, i.e. treat dashes and underscores as one and the same.
> 
> Right, I'll add a s/-/_/g on both ends of the strcmp or somesuch.

https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/commit/?h=module/namespace&id=f2aabf8436348a47037570af139ec2c1de8c5337

My test box seems able to load kvm_intel. I'll let it all sit in that
tree for a few days such that 0day might get a chance to chew on it
before posting.

Thanks!