| Message ID | 20241111111817.532312508@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | module: Strict per-modname namespaces | expand |
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.
--- 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); }
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(-)