On Thu, Nov 7, 2024 at 4:02 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > > Hi, > > I've been wanting $topic for a while, and having just stumbled into the > whole namespace thing by accident, I figured I'd give it a go, most if > the hard parts seem to have already been done. > > It reserves and disallows imports on any "MODULE_${name}" namespace, > while it implicitly adds the same namespace to every module. > > This allows exports targeted at specific modules and no others -- one > random example included. I've hated the various kvm exports we've had > for a while, and strictly limiting them to the kvm module helps > alleviate some abuse potential. > > --- > arch/x86/kernel/fpu/core.c | 2 +- > kernel/module/main.c | 28 ++++++++++++++++++++++++++-- > scripts/mod/modpost.c | 29 ++++++++++++++++++++--------- > 3 files changed, 47 insertions(+), 12 deletions(-) > > diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c > index 1209c7aebb21..23b188a53d9d 100644 > --- a/arch/x86/kernel/fpu/core.c > +++ b/arch/x86/kernel/fpu/core.c > @@ -366,7 +366,7 @@ int fpu_swap_kvm_fpstate(struct fpu_guest *guest_fpu, bool enter_guest) > fpregs_unlock(); > return 0; > } > -EXPORT_SYMBOL_GPL(fpu_swap_kvm_fpstate); > +EXPORT_SYMBOL_NS_GPL(fpu_swap_kvm_fpstate, MODULE_kvm); > > void fpu_copy_guest_fpstate_to_uabi(struct fpu_guest *gfpu, void *buf, > unsigned int size, u64 xfeatures, u32 pkru) > diff --git a/kernel/module/main.c b/kernel/module/main.c > index 49b9bca9de12..b30af879c2cb 100644 > --- a/kernel/module/main.c > +++ b/kernel/module/main.c > @@ -1070,6 +1070,13 @@ static int verify_namespace_is_imported(const struct load_info *info, > > 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(struct module *mod, const char *license) > } > } > > -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 *info, const char __user *uargs, > 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); > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 107393a8c48a..d1de3044ee03 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -1553,8 +1553,19 @@ static void mod_set_crcs(struct module *mod) > 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) > { > + char module_namespace[MODULE_NAME_LEN + 8]; > const char *symname; > char *version; > char *license; > @@ -1586,12 +1597,16 @@ static void read_symbols(const char *modname) > 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)) { The conversion from while() to for() is an unrelated change. Split it to a separate patch if you want to change it. > + if (strstarts(namespace, "MODULE_")) > + error("importing implicit module namespace: %s\n", namespace); > + > add_namespace(&mod->imported_namespaces, namespace); > - namespace = get_next_modinfo(&info, "import_ns", > - 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); > @@ -1700,11 +1715,7 @@ static void check_exports(struct module *mod) > 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); This is an unrelated change. So, it should be split into a separate prerequisite patch, something like, "modpost: introduce mod_basename() helper" > if (!contains_namespace(&mod->imported_namespaces, exp->namespace)) { > modpost_log(!allow_missing_ns_imports, -- Best Regards Masahiro Yamada