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

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.

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

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.