Symbol namespaces

The EXPORT_SYMBOL_NS() and EXPORT_SYMBOL_NS_GPL() macros can be used to export a symbol to a specific namespace. There are no _GPL_FUTURE and _UNUSED variants because these are currently unused, and I'm not sure they are necessary.

I didn't add EXPORT_SYMBOL_NS() for ASM exports; this patch sets the namespace of ASM exports to NULL by default. If there's a need, this should be pretty easy to add.

A module that wants to use a symbol exported to a namespace must add a MODULE_IMPORT_NS() statement to their module code; otherwise, modpost will complain when building the module, and the kernel module loader will warn when loading the module.

The ELF symbols are renamed to include the namespace with an asm label; for example, symbol 'usb_stor_suspend' in namespace USB_STORAGE becomes 'usb_stor_suspend.USB_STORAGE'. This allows modpost to do namespace checking, without having to go through all the effort of parsing ELF and reloction records just to get to the struct kernel_symbols.

On x86_64 I saw no difference in binary size (compression), but at runtime this will require a word of memory per export to hold the namespace. An alternative could be to store namespaced symbols in their own section and use a separate 'struct namespaced_kernel_symbol' for that section, at the cost of making the module loader more complex.

Signed-off-by Martijn Coenen <[email protected]>

---

include/asm-generic/export.h |  2 +-
include/linux/export.h       | 83 +++++++++++++++++++++++++++---------
include/linux/module.h       | 13 ++++++
kernel/module.c              | 79 ++++++++++++++++++++++++++++++++++
4 files changed, 156 insertions(+), 21 deletions(-)

include/asm-generic/export.h +0 -0

index 68efb950a918..4c3d1afb702f 100644
@@ -29,7 +29,7 @@
 	.section ___ksymtab\sec+\name,"a"
 	.balign KSYM_ALIGN

__ksymtab_\name:

include/asm-generic/export.h +1 -1

-	__put \val, __kstrtab_\name
+	__put \val, __kstrtab_\name, 0

.previous
.section __ksymtab_strings,"a"
__kstrtab_\name:

include/linux/export.h +63 -20

index ad6b8e697b27..9f6e70eeb85f 100644
@@ -22,6 +22,11 @@ struct kernel_symbol
 {
 	unsigned long value;
 	const char *name;
+	const char *namespace;
+};
+
+struct namespace_import {
+	const char *namespace;
 };
 
 #ifdef MODULE

@@ -54,18 +59,28 @@ extern struct module __this_module;
 #define __CRC_SYMBOL(sym, sec)
 #endif
 
+#define NS_SEPARATOR "."
+
+#define MODULE_IMPORT_NS(ns)						\
+	static const struct namespace_import __knsimport_##ns		\
+	asm("__knsimport_" #ns)						\
+	__attribute__((section("__knsimport"), used))			\
+	= { #ns }
+
 /* For every exported symbol, place a struct in the __ksymtab section */
-#define ___EXPORT_SYMBOL(sym, sec)					\
+#define ___EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)			\
 	extern typeof(sym) sym;						\
 	__CRC_SYMBOL(sym, sec)						\
-	static const char __kstrtab_##sym[]				\
+	static const char __kstrtab_##sym##nspost[]			\
 	__attribute__((section("__ksymtab_strings"), aligned(1)))	\
 	= #sym;								\
-	static const struct kernel_symbol __ksymtab_##sym		\
+	static const struct kernel_symbol __ksymtab_##sym##nspost	\
+	asm("__ksymtab_" #sym nspost2)                                  \
 	__used								\
-	__attribute__((section("___ksymtab" sec "+" #sym), used))	\
+	__attribute__((section("___ksymtab" sec "+" #sym #nspost)))	\
+	__attribute__((used))						\
 	__attribute__((aligned(sizeof(void *))))                        \
-	= { (unsigned long)&sym, __kstrtab_##sym }
+	= { (unsigned long)&sym, __kstrtab_##sym##nspost, ns }
 
 #if defined(__KSYM_DEPS__)
 

@@ -76,52 +91,80 @@ extern struct module __this_module;
 * system filters out from the preprocessor output (see ksym_dep_filter
 * in scripts/Kbuild.include).
 */
-#define __EXPORT_SYMBOL(sym, sec)	=== __KSYM_##sym ===
+#define __EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)	=== __KSYM_##sym ===
 
 #elif defined(CONFIG_TRIM_UNUSED_KSYMS)
 
 #include <generated/autoksyms.h>
 
-#define __EXPORT_SYMBOL(sym, sec)				\
-	__cond_export_sym(sym, sec, __is_defined(__KSYM_##sym))
-#define __cond_export_sym(sym, sec, conf)			\
-	___cond_export_sym(sym, sec, conf)
-#define ___cond_export_sym(sym, sec, enabled)			\
-	__cond_export_sym_##enabled(sym, sec)
-#define __cond_export_sym_1(sym, sec) ___EXPORT_SYMBOL(sym, sec)
-#define __cond_export_sym_0(sym, sec) /* nothing */
+#define __EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)			\
+	__cond_export_sym(sym, sec, ns, nspost, nspost2,		\
+			  __is_defined(__KSYM_##sym))
+#define __cond_export_sym(sym, sec, ns, nspost, nspost2, conf)		\
+	___cond_export_sym(sym, sec, ns, nspost, nspost2, conf)
+#define ___cond_export_sym(sym, sec, ns, nspost, nspost2, enabled)	\
+	__cond_export_sym_##enabled(sym, sec, ns, nspost, nspost2)
+#define __cond_export_sym_1(sym, sec, ns, nspost, nspost2)		\
+	___EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)
+#define __cond_export_sym_0(sym, sec, ns, nspost, nspost2) /* nothing */
 
 #else
 #define __EXPORT_SYMBOL ___EXPORT_SYMBOL
 #endif
 
 #define EXPORT_SYMBOL(sym)					\
-	__EXPORT_SYMBOL(sym, "")
+	__EXPORT_SYMBOL(sym, "", NULL, ,)
 
 #define EXPORT_SYMBOL_GPL(sym)					\
-	__EXPORT_SYMBOL(sym, "_gpl")
+	__EXPORT_SYMBOL(sym, "_gpl", NULL, ,)
 
 #define EXPORT_SYMBOL_GPL_FUTURE(sym)				\
-	__EXPORT_SYMBOL(sym, "_gpl_future")
+	__EXPORT_SYMBOL(sym, "_gpl_future", NULL, ,)
+
+#define EXPORT_SYMBOL_NS(sym, ns)                               \
+	__EXPORT_SYMBOL(sym, "", #ns, __##ns, NS_SEPARATOR #ns)
+
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)                               \
+	__EXPORT_SYMBOL(sym, "_gpl", #ns, __##ns, NS_SEPARATOR #ns)
 
 #ifdef CONFIG_UNUSED_SYMBOLS
-#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused")
-#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl")
+#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused", , ,)
+#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl", , ,)
 #else
 #define EXPORT_UNUSED_SYMBOL(sym)
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 #endif
 
-#endif	/* __GENKSYMS__ */
+#endif	/* __KERNEL__ && !__GENKSYMS__ */
+
+#if defined(__GENKSYMS__)
+/*
+ * When we're running genksyms, ignore the namespace and make the _NS
+ * variants look like the normal ones. There are two reasons for this:
+ * 1) In the normal definition of EXPORT_SYMBOL_NS, the 'ns' macro
+ *    argument is itself not expanded because it's always tokenized or
+ *    concatenated; but when running genksyms, a blank definition of the
+ *    macro does allow the argument to be expanded; if a namespace
+ *    happens to collide with a #define, this can cause issues.
+ * 2) There's no need to modify genksyms to deal with the _NS variants
+ */
+#define EXPORT_SYMBOL_NS(sym, ns)                              \
+	EXPORT_SYMBOL(sym)
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)                          \
+	EXPORT_SYMBOL_GPL(sym)
+#endif
 
 #else /* !CONFIG_MODULES... */
 
 #define EXPORT_SYMBOL(sym)
+#define EXPORT_SYMBOL_NS(sym, ns)
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)
 #define EXPORT_SYMBOL_GPL(sym)
 #define EXPORT_SYMBOL_GPL_FUTURE(sym)
 #define EXPORT_UNUSED_SYMBOL(sym)
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 
+#define MODULE_IMPORT_NS(ns)
 #endif /* CONFIG_MODULES */
 #endif /* !__ASSEMBLY__ */
 

include/linux/module.h +13 -0

index d44df9b2c131..afab4e8fa188 100644
@@ -268,6 +268,12 @@ void *__symbol_get(const char *symbol);
 void *__symbol_get_gpl(const char *symbol);
 #define symbol_get(x) ((typeof(&x))(__symbol_get(VMLINUX_SYMBOL_STR(x))))
 
+/* namespace dependencies of the module */
+struct module_ns_dep {
+	struct list_head ns_dep;
+	const char *namespace;
+};
+
 /* modules using other modules: kdb wants to see this. */
 struct module_use {
 	struct list_head source_list;

@@ -359,6 +365,13 @@ struct module {
 	const struct kernel_symbol *gpl_syms;
 	const s32 *gpl_crcs;
 
+	/* Namespace imports */
+	unsigned int num_ns_imports;
+	const struct namespace_import *ns_imports;
+
+	/* Namespace dependencies */
+	struct list_head ns_dependencies;
+
 #ifdef CONFIG_UNUSED_SYMBOLS
 	/* unused exported symbols. */
 	const struct kernel_symbol *unused_syms;

kernel/module.c +47 -0

index f475f30eed8c..63de0fe849f9 100644
@@ -1166,6 +1166,51 @@ static inline int module_unload_init(struct module *mod)
 }
 #endif /* CONFIG_MODULE_UNLOAD */
 
+static bool module_has_ns_dependency(struct module *mod, const char *ns)
+{
+	struct module_ns_dep *ns_dep;
+
+	list_for_each_entry(ns_dep, &mod->ns_dependencies, ns_dep) {
+		if (strcmp(ns_dep->namespace, ns) == 0)
+			return true;
+	}
+
+	return false;
+}
+
+static int add_module_ns_dependency(struct module *mod, const char *ns)
+{
+	struct module_ns_dep *ns_dep;
+
+	if (module_has_ns_dependency(mod, ns))
+		return 0;
+
+	ns_dep = kmalloc(sizeof(*ns_dep), GFP_ATOMIC);
+	if (!ns_dep)
+		return -ENOMEM;
+
+	ns_dep->namespace = ns;
+
+	list_add(&ns_dep->ns_dep, &mod->ns_dependencies);
+
+	return 0;
+}
+
+static bool module_imports_ns(struct module *mod, const char *ns)
+{
+	size_t i;
+
+	if (!ns)
+		return true;
+
+	for (i = 0; i < mod->num_ns_imports; ++i) {
+		if (!strcmp(mod->ns_imports[i].namespace, ns))
+			return true;
+	}
+
+	return false;
+}
+
 static size_t module_flags_taint(struct module *mod, char *buf)
 {
 	size_t l = 0;

@@ -1415,6 +1460,18 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
 		goto getname;
 	}
 
+	/*
+	 * We can't yet verify that the module actually imports this

getname:

	/* We must make copy under the lock if we failed to get ref. */
	strncpy(ownername, module_name(owner), MODULE_NAME_LEN);

kernel/module.c +22 -0

@@ -3061,6 +3118,11 @@ static int find_module_sections(struct module *mod, struct load_info *info)
 				     sizeof(*mod->gpl_syms),
 				     &mod->num_gpl_syms);
 	mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
+
+	mod->ns_imports = section_objs(info, "__knsimport",
+				       sizeof(*mod->ns_imports),
+				       &mod->num_ns_imports);
+
 	mod->gpl_future_syms = section_objs(info,
 					    "__ksymtab_gpl_future",
 					    sizeof(*mod->gpl_future_syms),

@@ -3381,6 +3443,19 @@ static int post_relocation(struct module *mod, const struct load_info *info)
 	return module_finalize(info->hdr, info->sechdrs, mod);
 }
 
+static void verify_namespace_dependencies(struct module *mod)
+{
+	struct module_ns_dep *ns_dep;
+
+	list_for_each_entry(ns_dep, &mod->ns_dependencies, ns_dep) {
+		if (!module_imports_ns(mod, ns_dep->namespace)) {
+			pr_warn("%s: module uses symbols from namespace %s,"
+				" but does not import it.\n",
+				mod->name, ns_dep->namespace);
+		}
+	}
+}
+
 /* Is this module of this name done loading?  No locks held. */
 static bool finished_loading(const char *name)
 {

@@ -3682,6 +3757,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	if (err)
 		goto free_module;
 
+	INIT_LIST_HEAD(&mod->ns_dependencies);
+
 #ifdef CONFIG_MODULE_SIG
 	mod->sig_ok = info->sig_ok;
 	if (!mod->sig_ok) {

@@ -3730,6 +3807,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	if (err < 0)
 		goto free_modinfo;
 
+	verify_namespace_dependencies(mod);
+
 	flush_module_icache(mod);
 
 	/* Now copy in args */

Signature · 1 line

2.18.0.203.gfac676dfb9-goog

First, thanks a lot for the patchset. This is definitely something that will help distros as well to declutter the exported symbol space and provide a more cleanly defined kernel interface.

Hm, I'm wondering if we could avoid all this extra module-ns-dependency checking work if we just put the import stuff as a modinfo tag, like have MODULE_IMPORT_NS() insert an "import:" tag like we already do for module parameters, author, license, aliases, etc. Then we'd have that information pretty much from the start of load_module() and we don't need to wait for relocations to be applied to __knsimport. Then you could do the namespace checking right at resolve_symbol() instead of going through the extra step of building a dependency list to be verified later.

Also, this would get rid of the extra __knsimport section, the extra ns_dependencies field in struct module, and all those helper functions that manage it. In addition, having the modinfo tag may potentially help with debugging as we have the namespace imports clearly listed if we don't have the source code for a module. We'd probably need to modify get_modinfo() to handle multiple import: tags though. Luis [1] had written some code a while ago to handle multiple (of the same) modinfo tags.

Thoughts on this?

Thanks,

Jessica

[1] https://lkml.kernel.org/r/[email protected]

Quoted 61 lines

getname:

        /* We must make copy under the lock if we failed to get ref. */
        strncpy(ownername, module_name(owner), MODULE_NAME_LEN);

kernel/module.c +5 -0

@@ -3061,6 +3118,11 @@ static int find_module_sections(struct module *mod,
 struct load_info *info)
                                     sizeof(*mod->gpl_syms),
                                     &mod->num_gpl_syms);
        mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
+
+       mod->ns_imports = section_objs(info, "__knsimport",
+                                      sizeof(*mod->ns_imports),
+                                      &mod->num_ns_imports);
+
        mod->gpl_future_syms = section_objs(info,
                                            "__ksymtab_gpl_future",

                                            sizeof(*mod->gpl_future_syms),

kernel/module.c +7 -0

@@ -3381,6 +3443,19 @@ static int post_relocation(struct module *mod,
 const struct load_info *info)
        return module_finalize(info->hdr, info->sechdrs, mod);
 }
 
+static void verify_namespace_dependencies(struct module *mod)
+{
+       struct module_ns_dep *ns_dep;
+
+       list_for_each_entry(ns_dep, &mod->ns_dependencies, ns_dep) {
+               if (!module_imports_ns(mod, ns_dep->namespace)) {
+                       pr_warn("%s: module uses symbols from namespace

%s,"

kernel/module.c +6 -0

+                               " but does not import it.\n",
+                               mod->name, ns_dep->namespace);
+               }
+       }
+}
+

/* Is this module of this name done loading? No locks held. */ static bool finished_loading(const char *name)
{

kernel/module.c +2 -0

@@ -3682,6 +3757,8 @@ static int load_module(struct load_info *info, const
 char __user *uargs,
        if (err)
                goto free_module;
 
+       INIT_LIST_HEAD(&mod->ns_dependencies);
+
 #ifdef CONFIG_MODULE_SIG
        mod->sig_ok = info->sig_ok;

        if (!mod->sig_ok) {

kernel/module.c +2 -0

@@ -3730,6 +3807,8 @@ static int load_module(struct load_info *info, const
 char __user *uargs,
        if (err < 0)
                goto free_modinfo;
 
+       verify_namespace_dependencies(mod);
+
        flush_module_icache(mod);
 

        /* Now copy in args */

--
2.18.0.203.gfac676dfb9-goog

Hi Jessica,

Quoted 3 lines · Jessica Yu

First, thanks a lot for the patchset. This is definitely something that will help distros as well to declutter the exported symbol space and provide a more cleanly defined kernel interface.

Thanks, that's good to hear!

Quoted 9 lines

Hm, I'm wondering if we could avoid all this extra module-ns-dependency checking work if we just put the import stuff as a modinfo tag, like have MODULE_IMPORT_NS() insert an "import:" tag like we already do for module parameters, author, license, aliases, etc. Then we'd have that information pretty much from the start of load_module() and we don't need to wait for relocations to be applied to __knsimport. Then you could do the namespace checking right at resolve_symbol() instead of going through the extra step of building a dependency list to be verified later.

I believe I did consider modinfo when I started with this a while back, but don't recall right now why I didn't decide to use it; perhaps it was the lack of support for multiple identical tags. Since both modpost and the module loader have access to them, I don't see why it wouldn't work, and indeed I suspect it would make the module loader a bit simpler. I'll rework this and see what it looks like.

Quoted 79 lines

Also, this would get rid of the extra __knsimport section, the extra ns_dependencies field in struct module, and all those helper functions that manage it. In addition, having the modinfo tag may potentially help with debugging as we have the namespace imports clearly listed if we don't have the source code for a module. We'd probably need to modify get_modinfo() to handle multiple import: tags though. Luis [1] had written some code a while ago to handle multiple (of the same) modinfo tags.

Thoughts on this?

Thanks,

Jessica

[1] https://lkml.kernel.org/r/[email protected]

Quoted 61 lines

getname:

        /* We must make copy under the lock if we failed to get ref. */
        strncpy(ownername, module_name(owner), MODULE_NAME_LEN);

kernel/module.c +5 -0

@@ -3061,6 +3118,11 @@ static int find_module_sections(struct module *mod,
 struct load_info *info)
                                     sizeof(*mod->gpl_syms),
                                     &mod->num_gpl_syms);
        mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
+
+       mod->ns_imports = section_objs(info, "__knsimport",
+                                      sizeof(*mod->ns_imports),
+                                      &mod->num_ns_imports);
+
        mod->gpl_future_syms = section_objs(info,
                                            "__ksymtab_gpl_future",

                                            sizeof(*mod->gpl_future_syms),

kernel/module.c +7 -0

@@ -3381,6 +3443,19 @@ static int post_relocation(struct module *mod,
 const struct load_info *info)
        return module_finalize(info->hdr, info->sechdrs, mod);
 }
 
+static void verify_namespace_dependencies(struct module *mod)
+{
+       struct module_ns_dep *ns_dep;
+
+       list_for_each_entry(ns_dep, &mod->ns_dependencies, ns_dep) {
+               if (!module_imports_ns(mod, ns_dep->namespace)) {
+                       pr_warn("%s: module uses symbols from namespace

%s,"

kernel/module.c +6 -0

+                               " but does not import it.\n",
+                               mod->name, ns_dep->namespace);
+               }
+       }
+}
+

/* Is this module of this name done loading? No locks held. */ static bool finished_loading(const char *name)
{

kernel/module.c +2 -0

@@ -3682,6 +3757,8 @@ static int load_module(struct load_info *info, const
 char __user *uargs,
        if (err)
                goto free_module;
 
+       INIT_LIST_HEAD(&mod->ns_dependencies);
+
 #ifdef CONFIG_MODULE_SIG
        mod->sig_ok = info->sig_ok;

        if (!mod->sig_ok) {

kernel/module.c +2 -0

@@ -3730,6 +3807,8 @@ static int load_module(struct load_info *info, const
 char __user *uargs,
        if (err < 0)
                goto free_modinfo;
 
+       verify_namespace_dependencies(mod);
+
        flush_module_icache(mod);
 

        /* Now copy in args */

--
2.18.0.203.gfac676dfb9-goog

Thanks. Also, it looks like we're currently just warning (in both modpost and on module load) if a module uses symbols from a namespace it doesn't import. Are you also planning to eventually introduce namespace enforcement?

It'd be trivial to fail the module build in
modpost as well as reject the module on load if it uses an exported symbol belonging to a namespace it doesn't import. Although, I'd go with the warnings for a development cycle or two, to gently introduce the change in API and let other subsystems as well as out-of-tree module developers catch up.

Jessica

Quoted 87 lines

Quoted 87 lines

Also, this would get rid of the extra __knsimport section, the extra ns_dependencies field in struct module, and all those helper functions that
manage it. In addition, having the modinfo tag may potentially help with debugging as we have the namespace imports clearly listed if we don't have
the source code for a module. We'd probably need to modify get_modinfo() to
handle multiple import: tags though. Luis [1] had written some code a while
ago to handle multiple (of the same) modinfo tags.

Thoughts on this?

Thanks,

Jessica

[1] https://lkml.kernel.org/r/[email protected]

Quoted 65 lines

getname:

        /* We must make copy under the lock if we failed to get ref. */
        strncpy(ownername, module_name(owner), MODULE_NAME_LEN);

kernel/module.c +5 -0

@@ -3061,6 +3118,11 @@ static int find_module_sections(struct module
 *mod,
 struct load_info *info)
                                     sizeof(*mod->gpl_syms),
                                     &mod->num_gpl_syms);
        mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
+
+       mod->ns_imports = section_objs(info, "__knsimport",
+                                      sizeof(*mod->ns_imports),
+                                      &mod->num_ns_imports);
+
        mod->gpl_future_syms = section_objs(info,

                                            "__ksymtab_gpl_future",

sizeof(*mod->gpl_future_syms),

kernel/module.c +7 -0

@@ -3381,6 +3443,19 @@ static int post_relocation(struct module *mod,
 const struct load_info *info)
        return module_finalize(info->hdr, info->sechdrs, mod);
 }
 
+static void verify_namespace_dependencies(struct module *mod)
+{
+       struct module_ns_dep *ns_dep;
+
+       list_for_each_entry(ns_dep, &mod->ns_dependencies, ns_dep) {
+               if (!module_imports_ns(mod, ns_dep->namespace)) {
+                       pr_warn("%s: module uses symbols from namespace

%s,"

kernel/module.c +6 -0

+                               " but does not import it.\n",
+                               mod->name, ns_dep->namespace);
+               }
+       }
+}
+

/* Is this module of this name done loading? No locks held. */ static bool finished_loading(const char *name)
{

(quoted diff) +0 -0

@@ -3682,6 +3757,8 @@ static int load_module(struct load_info *info,

const
char __user *uargs,

        if (err)
                goto free_module;

kernel/module.c +2 -0

+       INIT_LIST_HEAD(&mod->ns_dependencies);
+

#ifdef CONFIG_MODULE_SIG
        mod->sig_ok = info->sig_ok;
        if (!mod->sig_ok) {

(quoted diff) +0 -0

@@ -3730,6 +3807,8 @@ static int load_module(struct load_info *info,

const
char __user *uargs,

        if (err < 0)
                goto free_modinfo;

kernel/module.c +2 -1

+       verify_namespace_dependencies(mod);
+
        flush_module_icache(mod);
 
        /* Now copy in args */
--

2.18.0.203.gfac676dfb9-goog

Quoted 4 lines

Thanks. Also, it looks like we're currently just warning (in both modpost and on module load) if a module uses symbols from a namespace it doesn't import. Are you also planning to eventually introduce namespace enforcement?

This is something I've definitely been thinking about, and was curious what others would think. My main concern with enforcement is that it will start failing to load out-of-tree modules that use newly namespaced symbols. On the other hand, I think those modules will need to be rebuilt anyway to be able to load, because the changes to struct kernel_symbol make them incompatible with the current kernel. That could be another reason for having these symbols in a special section (with its own struct namespaced_kernel_symbol); but on the other hand, it would make the module loader more complex just to facilitate out-of-tree drivers, and I'm not sure where the trade-off between those two falls.

Quoted 6 lines

It'd be trivial to fail the module build in
modpost as well as reject the module on load if it uses an exported symbol belonging to a namespace it doesn't import. Although, I'd go with the warnings for a development cycle or two, to gently introduce the change in API and let other subsystems as well as out-of-tree module developers catch up.

An approach like that makes sense to me. Another alternative would be to make it a CONFIG_ option, and let distros/etc. decide what they are comfortable with.

IMO I don't think we should bend over backwards to accommodate out-of-tree modules - modifying the module loader to recognize even more special sections to accommodate these OOT modules would be where we'd draw the line I think.

I think going forward I would prefer to have export namespaces to be a normal and regular part of kernel API (as in, we shouldn't require a new option for it), and that the warnings for 1-2 cycles are courteous enough - but anyone with stronger opinions about this should speak up.

Thanks,

Jessica

The ELF symbols are renamed to include the namespace with an asm label; for example, symbol 'usb_stor_suspend' in namespace USB_STORAGE becomes 'usb_stor_suspend.USB_STORAGE'. This allows modpost to do namespace checking, without having to go through all the effort of parsing ELF and reloction records just to get to the struct kernel_symbols.

On x86_64 I saw no difference in binary size (compression), but at runtime this will require a word of memory per export to hold the namespace. An alternative could be to store namespaced symbols in their own section and use a separate 'struct namespaced_kernel_symbol' for that section, at the cost of making the module loader more complex.

Signed-off-by Martijn Coenen <[email protected]>

---
 include/asm-generic/export.h |  2 +-
 include/linux/export.h       | 83 +++++++++++++++++++++++++++---------
 include/linux/module.h       | 13 ++++++
 kernel/module.c              | 79 ++++++++++++++++++++++++++++++++++
 4 files changed, 156 insertions(+), 21 deletions(-)

include/asm-generic/export.h +1 -1

index 68efb950a918..4c3d1afb702f 100644
@@ -29,7 +29,7 @@
        .section ___ksymtab\sec+\name,"a"
        .balign KSYM_ALIGN
 __ksymtab_\name:
-       __put \val, __kstrtab_\name
+       __put \val, __kstrtab_\name, 0
        .previous
        .section __ksymtab_strings,"a"
 __kstrtab_\name:

include/linux/export.h +63 -20

index ad6b8e697b27..9f6e70eeb85f 100644
@@ -22,6 +22,11 @@ struct kernel_symbol
 {
        unsigned long value;
        const char *name;
+       const char *namespace;
+};
+
+struct namespace_import {
+       const char *namespace;
 };
 
 #ifdef MODULE

@@ -54,18 +59,28 @@ extern struct module __this_module;
 #define __CRC_SYMBOL(sym, sec)
 #endif
 
+#define NS_SEPARATOR "."
+
+#define MODULE_IMPORT_NS(ns)                                           \
+       static const struct namespace_import __knsimport_##ns           \
+       asm("__knsimport_" #ns)                                         \
+       __attribute__((section("__knsimport"), used))                   \
+       = { #ns }
+
 /* For every exported symbol, place a struct in the __ksymtab section */
-#define ___EXPORT_SYMBOL(sym, sec)                                     \
+#define ___EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)                        \
        extern typeof(sym) sym;                                         \
        __CRC_SYMBOL(sym, sec)                                          \
-       static const char __kstrtab_##sym[]                             \
+       static const char __kstrtab_##sym##nspost[]                     \
        __attribute__((section("__ksymtab_strings"), aligned(1)))       \
        = #sym;                                                         \
-       static const struct kernel_symbol __ksymtab_##sym               \
+       static const struct kernel_symbol __ksymtab_##sym##nspost       \
+       asm("__ksymtab_" #sym nspost2)                                  \
        __used                                                          \
-       __attribute__((section("___ksymtab" sec "+" #sym), used))       \
+       __attribute__((section("___ksymtab" sec "+" #sym #nspost)))     \
+       __attribute__((used))                                           \
        __attribute__((aligned(sizeof(void *))))                        \
-       = { (unsigned long)&sym, __kstrtab_##sym }
+       = { (unsigned long)&sym, __kstrtab_##sym##nspost, ns }
 
 #if defined(__KSYM_DEPS__)
 

@@ -76,52 +91,80 @@ extern struct module __this_module;
  * system filters out from the preprocessor output (see ksym_dep_filter
  * in scripts/Kbuild.include).
  */
-#define __EXPORT_SYMBOL(sym, sec)      === __KSYM_##sym ===
+#define __EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2) === __KSYM_##sym ===
 
 #elif defined(CONFIG_TRIM_UNUSED_KSYMS)
 
 #include <generated/autoksyms.h>
 
-#define __EXPORT_SYMBOL(sym, sec)                              \
-       __cond_export_sym(sym, sec, __is_defined(__KSYM_##sym))
-#define __cond_export_sym(sym, sec, conf)                      \
-       ___cond_export_sym(sym, sec, conf)
-#define ___cond_export_sym(sym, sec, enabled)                  \
-       __cond_export_sym_##enabled(sym, sec)
-#define __cond_export_sym_1(sym, sec) ___EXPORT_SYMBOL(sym, sec)
-#define __cond_export_sym_0(sym, sec) /* nothing */
+#define __EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)                 \
+       __cond_export_sym(sym, sec, ns, nspost, nspost2,                \
+                         __is_defined(__KSYM_##sym))
+#define __cond_export_sym(sym, sec, ns, nspost, nspost2, conf)         \
+       ___cond_export_sym(sym, sec, ns, nspost, nspost2, conf)
+#define ___cond_export_sym(sym, sec, ns, nspost, nspost2, enabled)     \
+       __cond_export_sym_##enabled(sym, sec, ns, nspost, nspost2)
+#define __cond_export_sym_1(sym, sec, ns, nspost, nspost2)             \
+       ___EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)
+#define __cond_export_sym_0(sym, sec, ns, nspost, nspost2) /* nothing */
 
 #else
 #define __EXPORT_SYMBOL ___EXPORT_SYMBOL
 #endif
 
 #define EXPORT_SYMBOL(sym)                                     \
-       __EXPORT_SYMBOL(sym, "")
+       __EXPORT_SYMBOL(sym, "", NULL, ,)
 
 #define EXPORT_SYMBOL_GPL(sym)                                 \
-       __EXPORT_SYMBOL(sym, "_gpl")
+       __EXPORT_SYMBOL(sym, "_gpl", NULL, ,)
 
 #define EXPORT_SYMBOL_GPL_FUTURE(sym)                          \
-       __EXPORT_SYMBOL(sym, "_gpl_future")
+       __EXPORT_SYMBOL(sym, "_gpl_future", NULL, ,)
+
+#define EXPORT_SYMBOL_NS(sym, ns)                               \
+       __EXPORT_SYMBOL(sym, "", #ns, __##ns, NS_SEPARATOR #ns)
+
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)                               \
+       __EXPORT_SYMBOL(sym, "_gpl", #ns, __##ns, NS_SEPARATOR #ns)
 
 #ifdef CONFIG_UNUSED_SYMBOLS
-#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused")
-#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl")
+#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused", , ,)
+#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl", , ,)
 #else
 #define EXPORT_UNUSED_SYMBOL(sym)
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 #endif
 
-#endif /* __GENKSYMS__ */
+#endif /* __KERNEL__ && !__GENKSYMS__ */
+
+#if defined(__GENKSYMS__)
+/*
+ * When we're running genksyms, ignore the namespace and make the _NS
+ * variants look like the normal ones. There are two reasons for this:
+ * 1) In the normal definition of EXPORT_SYMBOL_NS, the 'ns' macro
+ *    argument is itself not expanded because it's always tokenized or
+ *    concatenated; but when running genksyms, a blank definition of the
+ *    macro does allow the argument to be expanded; if a namespace
+ *    happens to collide with a #define, this can cause issues.
+ * 2) There's no need to modify genksyms to deal with the _NS variants
+ */
+#define EXPORT_SYMBOL_NS(sym, ns)                              \
+       EXPORT_SYMBOL(sym)
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)                          \
+       EXPORT_SYMBOL_GPL(sym)
+#endif
 
 #else /* !CONFIG_MODULES... */
 
 #define EXPORT_SYMBOL(sym)
+#define EXPORT_SYMBOL_NS(sym, ns)
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)
 #define EXPORT_SYMBOL_GPL(sym)
 #define EXPORT_SYMBOL_GPL_FUTURE(sym)
 #define EXPORT_UNUSED_SYMBOL(sym)
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 
+#define MODULE_IMPORT_NS(ns)
 #endif /* CONFIG_MODULES */
 #endif /* !__ASSEMBLY__ */
 

include/linux/module.h +13 -0

index d44df9b2c131..afab4e8fa188 100644
@@ -268,6 +268,12 @@ void *__symbol_get(const char *symbol);
 void *__symbol_get_gpl(const char *symbol);
 #define symbol_get(x) ((typeof(&x))(__symbol_get(VMLINUX_SYMBOL_STR(x))))
 
+/* namespace dependencies of the module */
+struct module_ns_dep {
+       struct list_head ns_dep;
+       const char *namespace;
+};
+
 /* modules using other modules: kdb wants to see this. */
 struct module_use {
        struct list_head source_list;

@@ -359,6 +365,13 @@ struct module {
        const struct kernel_symbol *gpl_syms;
        const s32 *gpl_crcs;
 
+       /* Namespace imports */
+       unsigned int num_ns_imports;
+       const struct namespace_import *ns_imports;
+
+       /* Namespace dependencies */
+       struct list_head ns_dependencies;
+
 #ifdef CONFIG_UNUSED_SYMBOLS
        /* unused exported symbols. */
        const struct kernel_symbol *unused_syms;

kernel/module.c +79 -1

index f475f30eed8c..63de0fe849f9 100644
@@ -1166,6 +1166,51 @@ static inline int module_unload_init(struct module *mod)
 }
 #endif /* CONFIG_MODULE_UNLOAD */
 
+static bool module_has_ns_dependency(struct module *mod, const char *ns)
+{
+       struct module_ns_dep *ns_dep;
+
+       list_for_each_entry(ns_dep, &mod->ns_dependencies, ns_dep) {
+               if (strcmp(ns_dep->namespace, ns) == 0)
+                       return true;
+       }
+
+       return false;
+}
+
+static int add_module_ns_dependency(struct module *mod, const char *ns)
+{
+       struct module_ns_dep *ns_dep;
+
+       if (module_has_ns_dependency(mod, ns))
+               return 0;
+
+       ns_dep = kmalloc(sizeof(*ns_dep), GFP_ATOMIC);
+       if (!ns_dep)
+               return -ENOMEM;
+
+       ns_dep->namespace = ns;
+
+       list_add(&ns_dep->ns_dep, &mod->ns_dependencies);
+
+       return 0;
+}
+
+static bool module_imports_ns(struct module *mod, const char *ns)
+{
+       size_t i;
+
+       if (!ns)
+               return true;
+
+       for (i = 0; i < mod->num_ns_imports; ++i) {
+               if (!strcmp(mod->ns_imports[i].namespace, ns))
+                       return true;
+       }
+
+       return false;
+}
+
 static size_t module_flags_taint(struct module *mod, char *buf)
 {
        size_t l = 0;

@@ -1415,6 +1460,18 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
                goto getname;
        }
 
+       /*
+        * We can't yet verify that the module actually imports this
+        * namespace, because the imports themselves are only available
+        * after processing the symbol table and doing relocation; so
+        * instead just record the dependency and check later.
+        */
+       if (sym->namespace) {
+               err = add_module_ns_dependency(mod, sym->namespace);
+               if (err)
+                       sym = ERR_PTR(err);
+       }
+
 getname:
        /* We must make copy under the lock if we failed to get ref. */
        strncpy(ownername, module_name(owner), MODULE_NAME_LEN);

@@ -3061,6 +3118,11 @@ static int find_module_sections(struct module *mod, struct load_info *info)
                                     sizeof(*mod->gpl_syms),
                                     &mod->num_gpl_syms);
        mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
+
+       mod->ns_imports = section_objs(info, "__knsimport",
+                                      sizeof(*mod->ns_imports),
+                                      &mod->num_ns_imports);
+
        mod->gpl_future_syms = section_objs(info,
                                            "__ksymtab_gpl_future",
                                            sizeof(*mod->gpl_future_syms),

@@ -3381,6 +3443,19 @@ static int post_relocation(struct module *mod, const struct load_info *info)
        return module_finalize(info->hdr, info->sechdrs, mod);
 }
 
+static void verify_namespace_dependencies(struct module *mod)
+{
+       struct module_ns_dep *ns_dep;
+
+       list_for_each_entry(ns_dep, &mod->ns_dependencies, ns_dep) {
+               if (!module_imports_ns(mod, ns_dep->namespace)) {
+                       pr_warn("%s: module uses symbols from namespace %s,"
+                               " but does not import it.\n",
+                               mod->name, ns_dep->namespace);
+               }
+       }
+}
+
 /* Is this module of this name done loading?  No locks held. */
 static bool finished_loading(const char *name)
 {

@@ -3682,6 +3757,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
        if (err)
                goto free_module;
 
+       INIT_LIST_HEAD(&mod->ns_dependencies);
+
 #ifdef CONFIG_MODULE_SIG
        mod->sig_ok = info->sig_ok;
        if (!mod->sig_ok) {

@@ -3730,6 +3807,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
        if (err < 0)
                goto free_modinfo;
 
+       verify_namespace_dependencies(mod);
+
        flush_module_icache(mod);
 
        /* Now copy in args */
--

2.18.0.203.gfac676dfb9-goog

I did find an issue with my approach:

Quoted 5 lines · Martijn Coenen

The ELF symbols are renamed to include the namespace with an asm label; for example, symbol 'usb_stor_suspend' in namespace USB_STORAGE becomes 'usb_stor_suspend.USB_STORAGE'. This allows modpost to do namespace checking, without having to go through all the effort of parsing ELF and reloction records just to get to the struct kernel_symbols.

depmod doesn't like this: if namespaced symbols are built-in to the kernel, they will appear as 'symbol.NS' in the symbol table, whereas modules using those symbols just depend on 'symbol'. This will cause depmod to warn about unknown symbols. I didn't find this previously because all the exports/imports I tested were done from modules themselves.

One way to deal with it is to change depmod, but it looks like it hasn't been changed in ages, and it would also introduce a dependency on userspaces to update it to avoid these warnings. So, we'd have to encode the symbol namespace information in another way for modpost to use. I could just write a separate post-processing tool (much like genksyms), or perhaps encode the information in a discardable section. Any other suggestions welcome.

Quoted 334 lines

On x86_64 I saw no difference in binary size (compression), but at runtime this will require a word of memory per export to hold the namespace. An alternative could be to store namespaced symbols in their own section and use a separate 'struct namespaced_kernel_symbol' for that section, at the cost of making the module loader more complex.

Signed-off-by Martijn Coenen <[email protected]>

---
 include/asm-generic/export.h |  2 +-
 include/linux/export.h       | 83 +++++++++++++++++++++++++++---------
 include/linux/module.h       | 13 ++++++
 kernel/module.c              | 79 ++++++++++++++++++++++++++++++++++
 4 files changed, 156 insertions(+), 21 deletions(-)

include/asm-generic/export.h +1 -1

index 68efb950a918..4c3d1afb702f 100644
@@ -29,7 +29,7 @@
        .section ___ksymtab\sec+\name,"a"
        .balign KSYM_ALIGN
 __ksymtab_\name:
-       __put \val, __kstrtab_\name
+       __put \val, __kstrtab_\name, 0
        .previous
        .section __ksymtab_strings,"a"
 __kstrtab_\name:

include/linux/export.h +63 -20

index ad6b8e697b27..9f6e70eeb85f 100644
@@ -22,6 +22,11 @@ struct kernel_symbol
 {
        unsigned long value;
        const char *name;
+       const char *namespace;
+};
+
+struct namespace_import {
+       const char *namespace;
 };
 
 #ifdef MODULE

@@ -54,18 +59,28 @@ extern struct module __this_module;
 #define __CRC_SYMBOL(sym, sec)
 #endif
 
+#define NS_SEPARATOR "."
+
+#define MODULE_IMPORT_NS(ns)                                           \
+       static const struct namespace_import __knsimport_##ns           \
+       asm("__knsimport_" #ns)                                         \
+       __attribute__((section("__knsimport"), used))                   \
+       = { #ns }
+
 /* For every exported symbol, place a struct in the __ksymtab section */
-#define ___EXPORT_SYMBOL(sym, sec)                                     \
+#define ___EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)                        \
        extern typeof(sym) sym;                                         \
        __CRC_SYMBOL(sym, sec)                                          \
-       static const char __kstrtab_##sym[]                             \
+       static const char __kstrtab_##sym##nspost[]                     \
        __attribute__((section("__ksymtab_strings"), aligned(1)))       \
        = #sym;                                                         \
-       static const struct kernel_symbol __ksymtab_##sym               \
+       static const struct kernel_symbol __ksymtab_##sym##nspost       \
+       asm("__ksymtab_" #sym nspost2)                                  \
        __used                                                          \
-       __attribute__((section("___ksymtab" sec "+" #sym), used))       \
+       __attribute__((section("___ksymtab" sec "+" #sym #nspost)))     \
+       __attribute__((used))                                           \
        __attribute__((aligned(sizeof(void *))))                        \
-       = { (unsigned long)&sym, __kstrtab_##sym }
+       = { (unsigned long)&sym, __kstrtab_##sym##nspost, ns }
 
 #if defined(__KSYM_DEPS__)
 

@@ -76,52 +91,80 @@ extern struct module __this_module;
  * system filters out from the preprocessor output (see ksym_dep_filter
  * in scripts/Kbuild.include).
  */
-#define __EXPORT_SYMBOL(sym, sec)      === __KSYM_##sym ===
+#define __EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2) === __KSYM_##sym ===
 
 #elif defined(CONFIG_TRIM_UNUSED_KSYMS)
 
 #include <generated/autoksyms.h>
 
-#define __EXPORT_SYMBOL(sym, sec)                              \
-       __cond_export_sym(sym, sec, __is_defined(__KSYM_##sym))
-#define __cond_export_sym(sym, sec, conf)                      \
-       ___cond_export_sym(sym, sec, conf)
-#define ___cond_export_sym(sym, sec, enabled)                  \
-       __cond_export_sym_##enabled(sym, sec)
-#define __cond_export_sym_1(sym, sec) ___EXPORT_SYMBOL(sym, sec)
-#define __cond_export_sym_0(sym, sec) /* nothing */
+#define __EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)                 \
+       __cond_export_sym(sym, sec, ns, nspost, nspost2,                \
+                         __is_defined(__KSYM_##sym))
+#define __cond_export_sym(sym, sec, ns, nspost, nspost2, conf)         \
+       ___cond_export_sym(sym, sec, ns, nspost, nspost2, conf)
+#define ___cond_export_sym(sym, sec, ns, nspost, nspost2, enabled)     \
+       __cond_export_sym_##enabled(sym, sec, ns, nspost, nspost2)
+#define __cond_export_sym_1(sym, sec, ns, nspost, nspost2)             \
+       ___EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)
+#define __cond_export_sym_0(sym, sec, ns, nspost, nspost2) /* nothing */
 
 #else
 #define __EXPORT_SYMBOL ___EXPORT_SYMBOL
 #endif
 
 #define EXPORT_SYMBOL(sym)                                     \
-       __EXPORT_SYMBOL(sym, "")
+       __EXPORT_SYMBOL(sym, "", NULL, ,)
 
 #define EXPORT_SYMBOL_GPL(sym)                                 \
-       __EXPORT_SYMBOL(sym, "_gpl")
+       __EXPORT_SYMBOL(sym, "_gpl", NULL, ,)
 
 #define EXPORT_SYMBOL_GPL_FUTURE(sym)                          \
-       __EXPORT_SYMBOL(sym, "_gpl_future")
+       __EXPORT_SYMBOL(sym, "_gpl_future", NULL, ,)
+
+#define EXPORT_SYMBOL_NS(sym, ns)                               \
+       __EXPORT_SYMBOL(sym, "", #ns, __##ns, NS_SEPARATOR #ns)
+
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)                               \
+       __EXPORT_SYMBOL(sym, "_gpl", #ns, __##ns, NS_SEPARATOR #ns)
 
 #ifdef CONFIG_UNUSED_SYMBOLS
-#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused")
-#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl")
+#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused", , ,)
+#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl", , ,)
 #else
 #define EXPORT_UNUSED_SYMBOL(sym)
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 #endif
 
-#endif /* __GENKSYMS__ */
+#endif /* __KERNEL__ && !__GENKSYMS__ */
+
+#if defined(__GENKSYMS__)
+/*
+ * When we're running genksyms, ignore the namespace and make the _NS
+ * variants look like the normal ones. There are two reasons for this:
+ * 1) In the normal definition of EXPORT_SYMBOL_NS, the 'ns' macro
+ *    argument is itself not expanded because it's always tokenized or
+ *    concatenated; but when running genksyms, a blank definition of the
+ *    macro does allow the argument to be expanded; if a namespace
+ *    happens to collide with a #define, this can cause issues.
+ * 2) There's no need to modify genksyms to deal with the _NS variants
+ */
+#define EXPORT_SYMBOL_NS(sym, ns)                              \
+       EXPORT_SYMBOL(sym)
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)                          \
+       EXPORT_SYMBOL_GPL(sym)
+#endif
 
 #else /* !CONFIG_MODULES... */
 
 #define EXPORT_SYMBOL(sym)
+#define EXPORT_SYMBOL_NS(sym, ns)
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)
 #define EXPORT_SYMBOL_GPL(sym)
 #define EXPORT_SYMBOL_GPL_FUTURE(sym)
 #define EXPORT_UNUSED_SYMBOL(sym)
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 
+#define MODULE_IMPORT_NS(ns)
 #endif /* CONFIG_MODULES */
 #endif /* !__ASSEMBLY__ */
 

include/linux/module.h +13 -0

index d44df9b2c131..afab4e8fa188 100644
@@ -268,6 +268,12 @@ void *__symbol_get(const char *symbol);
 void *__symbol_get_gpl(const char *symbol);
 #define symbol_get(x) ((typeof(&x))(__symbol_get(VMLINUX_SYMBOL_STR(x))))
 
+/* namespace dependencies of the module */
+struct module_ns_dep {
+       struct list_head ns_dep;
+       const char *namespace;
+};
+
 /* modules using other modules: kdb wants to see this. */
 struct module_use {
        struct list_head source_list;

@@ -359,6 +365,13 @@ struct module {
        const struct kernel_symbol *gpl_syms;
        const s32 *gpl_crcs;
 
+       /* Namespace imports */
+       unsigned int num_ns_imports;
+       const struct namespace_import *ns_imports;
+
+       /* Namespace dependencies */
+       struct list_head ns_dependencies;
+
 #ifdef CONFIG_UNUSED_SYMBOLS
        /* unused exported symbols. */
        const struct kernel_symbol *unused_syms;

kernel/module.c +79 -1

index f475f30eed8c..63de0fe849f9 100644
@@ -1166,6 +1166,51 @@ static inline int module_unload_init(struct module *mod)
 }
 #endif /* CONFIG_MODULE_UNLOAD */
 
+static bool module_has_ns_dependency(struct module *mod, const char *ns)
+{
+       struct module_ns_dep *ns_dep;
+
+       list_for_each_entry(ns_dep, &mod->ns_dependencies, ns_dep) {
+               if (strcmp(ns_dep->namespace, ns) == 0)
+                       return true;
+       }
+
+       return false;
+}
+
+static int add_module_ns_dependency(struct module *mod, const char *ns)
+{
+       struct module_ns_dep *ns_dep;
+
+       if (module_has_ns_dependency(mod, ns))
+               return 0;
+
+       ns_dep = kmalloc(sizeof(*ns_dep), GFP_ATOMIC);
+       if (!ns_dep)
+               return -ENOMEM;
+
+       ns_dep->namespace = ns;
+
+       list_add(&ns_dep->ns_dep, &mod->ns_dependencies);
+
+       return 0;
+}
+
+static bool module_imports_ns(struct module *mod, const char *ns)
+{
+       size_t i;
+
+       if (!ns)
+               return true;
+
+       for (i = 0; i < mod->num_ns_imports; ++i) {
+               if (!strcmp(mod->ns_imports[i].namespace, ns))
+                       return true;
+       }
+
+       return false;
+}
+
 static size_t module_flags_taint(struct module *mod, char *buf)
 {
        size_t l = 0;

@@ -1415,6 +1460,18 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
                goto getname;
        }
 
+       /*
+        * We can't yet verify that the module actually imports this
+        * namespace, because the imports themselves are only available
+        * after processing the symbol table and doing relocation; so
+        * instead just record the dependency and check later.
+        */
+       if (sym->namespace) {
+               err = add_module_ns_dependency(mod, sym->namespace);
+               if (err)
+                       sym = ERR_PTR(err);
+       }
+
 getname:
        /* We must make copy under the lock if we failed to get ref. */
        strncpy(ownername, module_name(owner), MODULE_NAME_LEN);

@@ -3061,6 +3118,11 @@ static int find_module_sections(struct module *mod, struct load_info *info)
                                     sizeof(*mod->gpl_syms),
                                     &mod->num_gpl_syms);
        mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
+
+       mod->ns_imports = section_objs(info, "__knsimport",
+                                      sizeof(*mod->ns_imports),
+                                      &mod->num_ns_imports);
+
        mod->gpl_future_syms = section_objs(info,
                                            "__ksymtab_gpl_future",
                                            sizeof(*mod->gpl_future_syms),

@@ -3381,6 +3443,19 @@ static int post_relocation(struct module *mod, const struct load_info *info)
        return module_finalize(info->hdr, info->sechdrs, mod);
 }
 
+static void verify_namespace_dependencies(struct module *mod)
+{
+       struct module_ns_dep *ns_dep;
+
+       list_for_each_entry(ns_dep, &mod->ns_dependencies, ns_dep) {
+               if (!module_imports_ns(mod, ns_dep->namespace)) {
+                       pr_warn("%s: module uses symbols from namespace %s,"
+                               " but does not import it.\n",
+                               mod->name, ns_dep->namespace);
+               }
+       }
+}
+
 /* Is this module of this name done loading?  No locks held. */
 static bool finished_loading(const char *name)
 {

@@ -3682,6 +3757,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
        if (err)
                goto free_module;
 
+       INIT_LIST_HEAD(&mod->ns_dependencies);
+
 #ifdef CONFIG_MODULE_SIG
        mod->sig_ok = info->sig_ok;
        if (!mod->sig_ok) {

@@ -3730,6 +3807,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
        if (err < 0)
                goto free_modinfo;
 
+       verify_namespace_dependencies(mod);
+
        flush_module_icache(mod);
 
        /* Now copy in args */
--

2.18.0.203.gfac676dfb9-goog

+++ Martijn Coenen [24/07/18 09:56 +0200]:

Quoted 18 lines

I did find an issue with my approach:

Quoted 5 lines · Martijn Coenen

The ELF symbols are renamed to include the namespace with an asm label; for example, symbol 'usb_stor_suspend' in namespace USB_STORAGE becomes 'usb_stor_suspend.USB_STORAGE'. This allows modpost to do namespace checking, without having to go through all the effort of parsing ELF and reloction records just to get to the struct kernel_symbols.

depmod doesn't like this: if namespaced symbols are built-in to the kernel, they will appear as 'symbol.NS' in the symbol table, whereas modules using those symbols just depend on 'symbol'. This will cause depmod to warn about unknown symbols. I didn't find this previously because all the exports/imports I tested were done from modules themselves.

One way to deal with it is to change depmod, but it looks like it hasn't been changed in ages, and it would also introduce a dependency

Quoted 5 lines

on userspaces to update it to avoid these warnings. So, we'd have to encode the symbol namespace information in another way for modpost to use. I could just write a separate post-processing tool (much like genksyms), or perhaps encode the information in a discardable section. Any other suggestions welcome.

This seems to be because depmod uses System.map as a source for kernel symbols and scans for only the ones prefixed with "__ksymtab" to find the exported ones - and those happen to use the '.' to mark the namespace it belongs to. It strips that prefix and uses the remainder as the actual symbol name. To fix that it'd have to strip the namespace suffix in the symbol name as well.

Just scanning the depmod source code, it seems really trivial to just have depmod accommodate the new symbol name format. Adding Lucas (kmod maintainer) to CC.

Thanks,

Jessica

Quoted 334 lines

Quoted 334 lines

On x86_64 I saw no difference in binary size (compression), but at runtime this will require a word of memory per export to hold the namespace. An alternative could be to store namespaced symbols in their own section and use a separate 'struct namespaced_kernel_symbol' for that section, at the cost of making the module loader more complex.

Signed-off-by Martijn Coenen <[email protected]>

---
 include/asm-generic/export.h |  2 +-
 include/linux/export.h       | 83 +++++++++++++++++++++++++++---------
 include/linux/module.h       | 13 ++++++
 kernel/module.c              | 79 ++++++++++++++++++++++++++++++++++
 4 files changed, 156 insertions(+), 21 deletions(-)

include/asm-generic/export.h +1 -1

index 68efb950a918..4c3d1afb702f 100644
@@ -29,7 +29,7 @@
        .section ___ksymtab\sec+\name,"a"
        .balign KSYM_ALIGN
 __ksymtab_\name:
-       __put \val, __kstrtab_\name
+       __put \val, __kstrtab_\name, 0
        .previous
        .section __ksymtab_strings,"a"
 __kstrtab_\name:

include/linux/export.h +63 -20

index ad6b8e697b27..9f6e70eeb85f 100644
@@ -22,6 +22,11 @@ struct kernel_symbol
 {
        unsigned long value;
        const char *name;
+       const char *namespace;
+};
+
+struct namespace_import {
+       const char *namespace;
 };
 
 #ifdef MODULE

@@ -54,18 +59,28 @@ extern struct module __this_module;
 #define __CRC_SYMBOL(sym, sec)
 #endif
 
+#define NS_SEPARATOR "."
+
+#define MODULE_IMPORT_NS(ns)                                           \
+       static const struct namespace_import __knsimport_##ns           \
+       asm("__knsimport_" #ns)                                         \
+       __attribute__((section("__knsimport"), used))                   \
+       = { #ns }
+
 /* For every exported symbol, place a struct in the __ksymtab section */
-#define ___EXPORT_SYMBOL(sym, sec)                                     \
+#define ___EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)                        \
        extern typeof(sym) sym;                                         \
        __CRC_SYMBOL(sym, sec)                                          \
-       static const char __kstrtab_##sym[]                             \
+       static const char __kstrtab_##sym##nspost[]                     \
        __attribute__((section("__ksymtab_strings"), aligned(1)))       \
        = #sym;                                                         \
-       static const struct kernel_symbol __ksymtab_##sym               \
+       static const struct kernel_symbol __ksymtab_##sym##nspost       \
+       asm("__ksymtab_" #sym nspost2)                                  \
        __used                                                          \
-       __attribute__((section("___ksymtab" sec "+" #sym), used))       \
+       __attribute__((section("___ksymtab" sec "+" #sym #nspost)))     \
+       __attribute__((used))                                           \
        __attribute__((aligned(sizeof(void *))))                        \
-       = { (unsigned long)&sym, __kstrtab_##sym }
+       = { (unsigned long)&sym, __kstrtab_##sym##nspost, ns }
 
 #if defined(__KSYM_DEPS__)
 

@@ -76,52 +91,80 @@ extern struct module __this_module;
  * system filters out from the preprocessor output (see ksym_dep_filter
  * in scripts/Kbuild.include).
  */
-#define __EXPORT_SYMBOL(sym, sec)      === __KSYM_##sym ===
+#define __EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2) === __KSYM_##sym ===
 
 #elif defined(CONFIG_TRIM_UNUSED_KSYMS)
 
 #include <generated/autoksyms.h>
 
-#define __EXPORT_SYMBOL(sym, sec)                              \
-       __cond_export_sym(sym, sec, __is_defined(__KSYM_##sym))
-#define __cond_export_sym(sym, sec, conf)                      \
-       ___cond_export_sym(sym, sec, conf)
-#define ___cond_export_sym(sym, sec, enabled)                  \
-       __cond_export_sym_##enabled(sym, sec)
-#define __cond_export_sym_1(sym, sec) ___EXPORT_SYMBOL(sym, sec)
-#define __cond_export_sym_0(sym, sec) /* nothing */
+#define __EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)                 \
+       __cond_export_sym(sym, sec, ns, nspost, nspost2,                \
+                         __is_defined(__KSYM_##sym))
+#define __cond_export_sym(sym, sec, ns, nspost, nspost2, conf)         \
+       ___cond_export_sym(sym, sec, ns, nspost, nspost2, conf)
+#define ___cond_export_sym(sym, sec, ns, nspost, nspost2, enabled)     \
+       __cond_export_sym_##enabled(sym, sec, ns, nspost, nspost2)
+#define __cond_export_sym_1(sym, sec, ns, nspost, nspost2)             \
+       ___EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)
+#define __cond_export_sym_0(sym, sec, ns, nspost, nspost2) /* nothing */
 
 #else
 #define __EXPORT_SYMBOL ___EXPORT_SYMBOL
 #endif
 
 #define EXPORT_SYMBOL(sym)                                     \
-       __EXPORT_SYMBOL(sym, "")
+       __EXPORT_SYMBOL(sym, "", NULL, ,)
 
 #define EXPORT_SYMBOL_GPL(sym)                                 \
-       __EXPORT_SYMBOL(sym, "_gpl")
+       __EXPORT_SYMBOL(sym, "_gpl", NULL, ,)
 
 #define EXPORT_SYMBOL_GPL_FUTURE(sym)                          \
-       __EXPORT_SYMBOL(sym, "_gpl_future")
+       __EXPORT_SYMBOL(sym, "_gpl_future", NULL, ,)
+
+#define EXPORT_SYMBOL_NS(sym, ns)                               \
+       __EXPORT_SYMBOL(sym, "", #ns, __##ns, NS_SEPARATOR #ns)
+
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)                               \
+       __EXPORT_SYMBOL(sym, "_gpl", #ns, __##ns, NS_SEPARATOR #ns)
 
 #ifdef CONFIG_UNUSED_SYMBOLS
-#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused")
-#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl")
+#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused", , ,)
+#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl", , ,)
 #else
 #define EXPORT_UNUSED_SYMBOL(sym)
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 #endif
 
-#endif /* __GENKSYMS__ */
+#endif /* __KERNEL__ && !__GENKSYMS__ */
+
+#if defined(__GENKSYMS__)
+/*
+ * When we're running genksyms, ignore the namespace and make the _NS
+ * variants look like the normal ones. There are two reasons for this:
+ * 1) In the normal definition of EXPORT_SYMBOL_NS, the 'ns' macro
+ *    argument is itself not expanded because it's always tokenized or
+ *    concatenated; but when running genksyms, a blank definition of the
+ *    macro does allow the argument to be expanded; if a namespace
+ *    happens to collide with a #define, this can cause issues.
+ * 2) There's no need to modify genksyms to deal with the _NS variants
+ */
+#define EXPORT_SYMBOL_NS(sym, ns)                              \
+       EXPORT_SYMBOL(sym)
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)                          \
+       EXPORT_SYMBOL_GPL(sym)
+#endif
 
 #else /* !CONFIG_MODULES... */
 
 #define EXPORT_SYMBOL(sym)
+#define EXPORT_SYMBOL_NS(sym, ns)
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)
 #define EXPORT_SYMBOL_GPL(sym)
 #define EXPORT_SYMBOL_GPL_FUTURE(sym)
 #define EXPORT_UNUSED_SYMBOL(sym)
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 
+#define MODULE_IMPORT_NS(ns)
 #endif /* CONFIG_MODULES */
 #endif /* !__ASSEMBLY__ */
 

include/linux/module.h +13 -0

index d44df9b2c131..afab4e8fa188 100644
@@ -268,6 +268,12 @@ void *__symbol_get(const char *symbol);
 void *__symbol_get_gpl(const char *symbol);
 #define symbol_get(x) ((typeof(&x))(__symbol_get(VMLINUX_SYMBOL_STR(x))))
 
+/* namespace dependencies of the module */
+struct module_ns_dep {
+       struct list_head ns_dep;
+       const char *namespace;
+};
+
 /* modules using other modules: kdb wants to see this. */
 struct module_use {
        struct list_head source_list;

@@ -359,6 +365,13 @@ struct module {
        const struct kernel_symbol *gpl_syms;
        const s32 *gpl_crcs;
 
+       /* Namespace imports */
+       unsigned int num_ns_imports;
+       const struct namespace_import *ns_imports;
+
+       /* Namespace dependencies */
+       struct list_head ns_dependencies;
+
 #ifdef CONFIG_UNUSED_SYMBOLS
        /* unused exported symbols. */
        const struct kernel_symbol *unused_syms;

kernel/module.c +79 -1

index f475f30eed8c..63de0fe849f9 100644
@@ -1166,6 +1166,51 @@ static inline int module_unload_init(struct module *mod)
 }
 #endif /* CONFIG_MODULE_UNLOAD */
 
+static bool module_has_ns_dependency(struct module *mod, const char *ns)
+{
+       struct module_ns_dep *ns_dep;
+
+       list_for_each_entry(ns_dep, &mod->ns_dependencies, ns_dep) {
+               if (strcmp(ns_dep->namespace, ns) == 0)
+                       return true;
+       }
+
+       return false;
+}
+
+static int add_module_ns_dependency(struct module *mod, const char *ns)
+{
+       struct module_ns_dep *ns_dep;
+
+       if (module_has_ns_dependency(mod, ns))
+               return 0;
+
+       ns_dep = kmalloc(sizeof(*ns_dep), GFP_ATOMIC);
+       if (!ns_dep)
+               return -ENOMEM;
+
+       ns_dep->namespace = ns;
+
+       list_add(&ns_dep->ns_dep, &mod->ns_dependencies);
+
+       return 0;
+}
+
+static bool module_imports_ns(struct module *mod, const char *ns)
+{
+       size_t i;
+
+       if (!ns)
+               return true;
+
+       for (i = 0; i < mod->num_ns_imports; ++i) {
+               if (!strcmp(mod->ns_imports[i].namespace, ns))
+                       return true;
+       }
+
+       return false;
+}
+
 static size_t module_flags_taint(struct module *mod, char *buf)
 {
        size_t l = 0;

@@ -1415,6 +1460,18 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
                goto getname;
        }
 
+       /*
+        * We can't yet verify that the module actually imports this
+        * namespace, because the imports themselves are only available
+        * after processing the symbol table and doing relocation; so
+        * instead just record the dependency and check later.
+        */
+       if (sym->namespace) {
+               err = add_module_ns_dependency(mod, sym->namespace);
+               if (err)
+                       sym = ERR_PTR(err);
+       }
+
 getname:
        /* We must make copy under the lock if we failed to get ref. */
        strncpy(ownername, module_name(owner), MODULE_NAME_LEN);

@@ -3061,6 +3118,11 @@ static int find_module_sections(struct module *mod, struct load_info *info)
                                     sizeof(*mod->gpl_syms),
                                     &mod->num_gpl_syms);
        mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
+
+       mod->ns_imports = section_objs(info, "__knsimport",
+                                      sizeof(*mod->ns_imports),
+                                      &mod->num_ns_imports);
+
        mod->gpl_future_syms = section_objs(info,
                                            "__ksymtab_gpl_future",
                                            sizeof(*mod->gpl_future_syms),

@@ -3381,6 +3443,19 @@ static int post_relocation(struct module *mod, const struct load_info *info)
        return module_finalize(info->hdr, info->sechdrs, mod);
 }
 
+static void verify_namespace_dependencies(struct module *mod)
+{
+       struct module_ns_dep *ns_dep;
+
+       list_for_each_entry(ns_dep, &mod->ns_dependencies, ns_dep) {
+               if (!module_imports_ns(mod, ns_dep->namespace)) {
+                       pr_warn("%s: module uses symbols from namespace %s,"
+                               " but does not import it.\n",
+                               mod->name, ns_dep->namespace);
+               }
+       }
+}
+
 /* Is this module of this name done loading?  No locks held. */
 static bool finished_loading(const char *name)
 {

@@ -3682,6 +3757,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
        if (err)
                goto free_module;
 
+       INIT_LIST_HEAD(&mod->ns_dependencies);
+
 #ifdef CONFIG_MODULE_SIG
        mod->sig_ok = info->sig_ok;
        if (!mod->sig_ok) {

@@ -3730,6 +3807,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
        if (err < 0)
                goto free_modinfo;
 
+       verify_namespace_dependencies(mod);
+
        flush_module_icache(mod);
 
        /* Now copy in args */
--

2.18.0.203.gfac676dfb9-goog

this might be because you are looking to the wrong project (module-init-tools) rather than kmod that replaced it some years ago?

Quoted 3 lines

Just scanning the depmod source code, it seems really trivial to just have depmod accommodate the new symbol name format. Adding Lucas (kmod maintainer) to CC.

Yep, that would be easy and allow depmod to be backward compatible since we would do nothing if the symbol doesn't have the suffix. As for dependency on a new version, this seems trivial enough to be backported to previous releases used on distros so even if they are not rolling they would get a compatible depmod.

CC'ing [email protected] to keep track of this there

thanks
Lucas De Marchi

Quoted 331 lines

Thanks,

Jessica

Quoted 326 lines

Quoted 326 lines

On x86_64 I saw no difference in binary size (compression), but at runtime this will require a word of memory per export to hold the namespace. An alternative could be to store namespaced symbols in their own section and use a separate 'struct namespaced_kernel_symbol' for that section, at the cost of making the module loader more complex.

Signed-off-by Martijn Coenen <[email protected]>

---
 include/asm-generic/export.h |  2 +-
 include/linux/export.h       | 83 +++++++++++++++++++++++++++---------
 include/linux/module.h       | 13 ++++++
 kernel/module.c              | 79 ++++++++++++++++++++++++++++++++++
 4 files changed, 156 insertions(+), 21 deletions(-)

include/asm-generic/export.h +1 -1

index 68efb950a918..4c3d1afb702f 100644
@@ -29,7 +29,7 @@
        .section ___ksymtab\sec+\name,"a"
        .balign KSYM_ALIGN
 __ksymtab_\name:
-       __put \val, __kstrtab_\name
+       __put \val, __kstrtab_\name, 0
        .previous
        .section __ksymtab_strings,"a"
 __kstrtab_\name:

include/linux/export.h +63 -20

index ad6b8e697b27..9f6e70eeb85f 100644
@@ -22,6 +22,11 @@ struct kernel_symbol
 {
        unsigned long value;
        const char *name;
+       const char *namespace;
+};
+
+struct namespace_import {
+       const char *namespace;
 };
 
 #ifdef MODULE

@@ -54,18 +59,28 @@ extern struct module __this_module;
 #define __CRC_SYMBOL(sym, sec)
 #endif
 
+#define NS_SEPARATOR "."
+
+#define MODULE_IMPORT_NS(ns)                                           \
+       static const struct namespace_import __knsimport_##ns           \
+       asm("__knsimport_" #ns)                                         \
+       __attribute__((section("__knsimport"), used))                   \
+       = { #ns }
+
 /* For every exported symbol, place a struct in the __ksymtab section */
-#define ___EXPORT_SYMBOL(sym, sec)                                     \
+#define ___EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)                        \
        extern typeof(sym) sym;                                         \
        __CRC_SYMBOL(sym, sec)                                          \
-       static const char __kstrtab_##sym[]                             \
+       static const char __kstrtab_##sym##nspost[]                     \
        __attribute__((section("__ksymtab_strings"), aligned(1)))       \
        = #sym;                                                         \
-       static const struct kernel_symbol __ksymtab_##sym               \
+       static const struct kernel_symbol __ksymtab_##sym##nspost       \
+       asm("__ksymtab_" #sym nspost2)                                  \
        __used                                                          \
-       __attribute__((section("___ksymtab" sec "+" #sym), used))       \
+       __attribute__((section("___ksymtab" sec "+" #sym #nspost)))     \
+       __attribute__((used))                                           \
        __attribute__((aligned(sizeof(void *))))                        \
-       = { (unsigned long)&sym, __kstrtab_##sym }
+       = { (unsigned long)&sym, __kstrtab_##sym##nspost, ns }
 
 #if defined(__KSYM_DEPS__)
 

@@ -76,52 +91,80 @@ extern struct module __this_module;
  * system filters out from the preprocessor output (see ksym_dep_filter
  * in scripts/Kbuild.include).
  */
-#define __EXPORT_SYMBOL(sym, sec)      === __KSYM_##sym ===
+#define __EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2) === __KSYM_##sym ===
 
 #elif defined(CONFIG_TRIM_UNUSED_KSYMS)
 
 #include <generated/autoksyms.h>
 
-#define __EXPORT_SYMBOL(sym, sec)                              \
-       __cond_export_sym(sym, sec, __is_defined(__KSYM_##sym))
-#define __cond_export_sym(sym, sec, conf)                      \
-       ___cond_export_sym(sym, sec, conf)
-#define ___cond_export_sym(sym, sec, enabled)                  \
-       __cond_export_sym_##enabled(sym, sec)
-#define __cond_export_sym_1(sym, sec) ___EXPORT_SYMBOL(sym, sec)
-#define __cond_export_sym_0(sym, sec) /* nothing */
+#define __EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)                 \
+       __cond_export_sym(sym, sec, ns, nspost, nspost2,                \
+                         __is_defined(__KSYM_##sym))
+#define __cond_export_sym(sym, sec, ns, nspost, nspost2, conf)         \
+       ___cond_export_sym(sym, sec, ns, nspost, nspost2, conf)
+#define ___cond_export_sym(sym, sec, ns, nspost, nspost2, enabled)     \
+       __cond_export_sym_##enabled(sym, sec, ns, nspost, nspost2)
+#define __cond_export_sym_1(sym, sec, ns, nspost, nspost2)             \
+       ___EXPORT_SYMBOL(sym, sec, ns, nspost, nspost2)
+#define __cond_export_sym_0(sym, sec, ns, nspost, nspost2) /* nothing */
 
 #else
 #define __EXPORT_SYMBOL ___EXPORT_SYMBOL
 #endif
 
 #define EXPORT_SYMBOL(sym)                                     \
-       __EXPORT_SYMBOL(sym, "")
+       __EXPORT_SYMBOL(sym, "", NULL, ,)
 
 #define EXPORT_SYMBOL_GPL(sym)                                 \
-       __EXPORT_SYMBOL(sym, "_gpl")
+       __EXPORT_SYMBOL(sym, "_gpl", NULL, ,)
 
 #define EXPORT_SYMBOL_GPL_FUTURE(sym)                          \
-       __EXPORT_SYMBOL(sym, "_gpl_future")
+       __EXPORT_SYMBOL(sym, "_gpl_future", NULL, ,)
+
+#define EXPORT_SYMBOL_NS(sym, ns)                               \
+       __EXPORT_SYMBOL(sym, "", #ns, __##ns, NS_SEPARATOR #ns)
+
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)                               \
+       __EXPORT_SYMBOL(sym, "_gpl", #ns, __##ns, NS_SEPARATOR #ns)
 
 #ifdef CONFIG_UNUSED_SYMBOLS
-#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused")
-#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl")
+#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused", , ,)
+#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl", , ,)
 #else
 #define EXPORT_UNUSED_SYMBOL(sym)
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 #endif
 
-#endif /* __GENKSYMS__ */
+#endif /* __KERNEL__ && !__GENKSYMS__ */
+
+#if defined(__GENKSYMS__)
+/*
+ * When we're running genksyms, ignore the namespace and make the _NS
+ * variants look like the normal ones. There are two reasons for this:
+ * 1) In the normal definition of EXPORT_SYMBOL_NS, the 'ns' macro
+ *    argument is itself not expanded because it's always tokenized or
+ *    concatenated; but when running genksyms, a blank definition of the
+ *    macro does allow the argument to be expanded; if a namespace
+ *    happens to collide with a #define, this can cause issues.
+ * 2) There's no need to modify genksyms to deal with the _NS variants
+ */
+#define EXPORT_SYMBOL_NS(sym, ns)                              \
+       EXPORT_SYMBOL(sym)
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)                          \
+       EXPORT_SYMBOL_GPL(sym)
+#endif
 
 #else /* !CONFIG_MODULES... */
 
 #define EXPORT_SYMBOL(sym)
+#define EXPORT_SYMBOL_NS(sym, ns)
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)
 #define EXPORT_SYMBOL_GPL(sym)
 #define EXPORT_SYMBOL_GPL_FUTURE(sym)
 #define EXPORT_UNUSED_SYMBOL(sym)
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 
+#define MODULE_IMPORT_NS(ns)
 #endif /* CONFIG_MODULES */
 #endif /* !__ASSEMBLY__ */
 

include/linux/module.h +13 -0

index d44df9b2c131..afab4e8fa188 100644
@@ -268,6 +268,12 @@ void *__symbol_get(const char *symbol);
 void *__symbol_get_gpl(const char *symbol);
 #define symbol_get(x) ((typeof(&x))(__symbol_get(VMLINUX_SYMBOL_STR(x))))
 
+/* namespace dependencies of the module */
+struct module_ns_dep {
+       struct list_head ns_dep;
+       const char *namespace;
+};
+
 /* modules using other modules: kdb wants to see this. */
 struct module_use {
        struct list_head source_list;

@@ -359,6 +365,13 @@ struct module {
        const struct kernel_symbol *gpl_syms;
        const s32 *gpl_crcs;
 
+       /* Namespace imports */
+       unsigned int num_ns_imports;
+       const struct namespace_import *ns_imports;
+
+       /* Namespace dependencies */
+       struct list_head ns_dependencies;
+
 #ifdef CONFIG_UNUSED_SYMBOLS
        /* unused exported symbols. */
        const struct kernel_symbol *unused_syms;

kernel/module.c +77 -0

index f475f30eed8c..63de0fe849f9 100644
@@ -1166,6 +1166,51 @@ static inline int module_unload_init(struct module *mod)
 }
 #endif /* CONFIG_MODULE_UNLOAD */
 
+static bool module_has_ns_dependency(struct module *mod, const char *ns)
+{
+       struct module_ns_dep *ns_dep;
+
+       list_for_each_entry(ns_dep, &mod->ns_dependencies, ns_dep) {
+               if (strcmp(ns_dep->namespace, ns) == 0)
+                       return true;
+       }
+
+       return false;
+}
+
+static int add_module_ns_dependency(struct module *mod, const char *ns)
+{
+       struct module_ns_dep *ns_dep;
+
+       if (module_has_ns_dependency(mod, ns))
+               return 0;
+
+       ns_dep = kmalloc(sizeof(*ns_dep), GFP_ATOMIC);
+       if (!ns_dep)
+               return -ENOMEM;
+
+       ns_dep->namespace = ns;
+
+       list_add(&ns_dep->ns_dep, &mod->ns_dependencies);
+
+       return 0;
+}
+
+static bool module_imports_ns(struct module *mod, const char *ns)
+{
+       size_t i;
+
+       if (!ns)
+               return true;
+
+       for (i = 0; i < mod->num_ns_imports; ++i) {
+               if (!strcmp(mod->ns_imports[i].namespace, ns))
+                       return true;
+       }
+
+       return false;
+}
+
 static size_t module_flags_taint(struct module *mod, char *buf)
 {
        size_t l = 0;

@@ -1415,6 +1460,18 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
                goto getname;
        }
 
+       /*
+        * We can't yet verify that the module actually imports this
+        * namespace, because the imports themselves are only available
+        * after processing the symbol table and doing relocation; so
+        * instead just record the dependency and check later.
+        */
+       if (sym->namespace) {
+               err = add_module_ns_dependency(mod, sym->namespace);
+               if (err)
+                       sym = ERR_PTR(err);
+       }
+
 getname:
        /* We must make copy under the lock if we failed to get ref. */
        strncpy(ownername, module_name(owner), MODULE_NAME_LEN);

@@ -3061,6 +3118,11 @@ static int find_module_sections(struct module *mod, struct load_info *info)
                                     sizeof(*mod->gpl_syms),
                                     &mod->num_gpl_syms);
        mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
+
+       mod->ns_imports = section_objs(info, "__knsimport",
+                                      sizeof(*mod->ns_imports),
+                                      &mod->num_ns_imports);
+
        mod->gpl_future_syms = section_objs(info,
                                            "__ksymtab_gpl_future",
                                            sizeof(*mod->gpl_future_syms),

@@ -3381,6 +3443,19 @@ static int post_relocation(struct module *mod, const struct load_info *info)
        return module_finalize(info->hdr, info->sechdrs, mod);
 }
 
+static void verify_namespace_dependencies(struct module *mod)
+{
+       struct module_ns_dep *ns_dep;
+
+       list_for_each_entry(ns_dep, &mod->ns_dependencies, ns_dep) {
+               if (!module_imports_ns(mod, ns_dep->namespace)) {
+                       pr_warn("%s: module uses symbols from namespace %s,"
+                               " but does not import it.\n",
+                               mod->name, ns_dep->namespace);
+               }
+       }
+}
+
 /* Is this module of this name done loading?  No locks held. */
 static bool finished_loading(const char *name)
 {

@@ -3682,6 +3757,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
        if (err)
                goto free_module;
 
+       INIT_LIST_HEAD(&mod->ns_dependencies);
+
 #ifdef CONFIG_MODULE_SIG
        mod->sig_ok = info->sig_ok;
        if (!mod->sig_ok) {

@@ -3730,6 +3807,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
        if (err < 0)
                goto free_modinfo;
 

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

kernel/module.c +2 -1

+       verify_namespace_dependencies(mod);
+
        flush_module_icache(mod);
 
        /* Now copy in args */
--

2.18.0.203.gfac676dfb9-goog

--
Lucas De Marchi

This patch adds an option to modpost to generate a .ns_deps file per module, containing the namespace depedencies for that module.

This file can subsequently be used by other tools to automatically add newly introduced namespaces to the modules that require them, saving a lot of manual work.

Signed-off-by Martijn Coenen <[email protected]>

---
scripts/mod/modpost.c | 58 +++++++++++++++++++++++++++++++++++++++---- scripts/mod/modpost.h | 2 ++

2 files changed, 55 insertions(+), 5 deletions(-)

scripts/mod/modpost.c +53 -5

index a56a8461a96a..be33d60d5527 100644
@@ -39,6 +39,8 @@ static int sec_mismatch_verbose = 1;
 static int sec_mismatch_fatal = 0;
 /* ignore missing files */
 static int ignore_missing_files;
+/* Write namespace dependencies */
+static int write_ns_deps;
 
 enum export {
 	export_plain,      export_unused,     export_gpl,

@@ -2151,10 +2153,15 @@ static void check_exports(struct module *mod)
 		else
 			basename = mod->name;
 
-		if (exp->ns && !module_imports_namespace(mod, exp->ns)) {
-			warn("module %s uses symbol %s from namespace %s, "
-			     "but does not import it.\n",
-			     basename, exp->name, exp->ns);
+		if (exp->ns) {
+			add_namespace(&mod->required_namespaces, exp->ns);
+
+			if (!write_ns_deps &&
+			    !module_imports_namespace(mod, exp->ns)) {
+				warn("module %s uses symbol %s from namespace "
+				     "%s, but does not import it.\n",
+				     basename, exp->name, exp->ns);
+			}
 		}
 
 		if (!mod->gpl_compatible)

@@ -2457,6 +2464,38 @@ static void write_dump(const char *fname)
 	free(buf.p);
 }
 
+static void write_ns_deps_files(void)
+{
+	struct module *mod;
+	struct namespace_list *ns;
+	struct buffer ns_deps_buf = { };
+
+	for (mod = modules; mod; mod = mod->next) {
+		char fname[PATH_MAX];
+		const char *basename;
+
+		if (mod->skip)
+			continue;
+
+		ns_deps_buf.pos = 0;
+
+		for (ns = mod->required_namespaces; ns; ns = ns->next)
+			buf_printf(&ns_deps_buf, "%s\n", ns->namespace);
+
+		if (ns_deps_buf.pos == 0)
+			continue;
+
+		basename = strrchr(mod->name, '/');
+		if (basename)
+			basename++;
+		else
+			basename = mod->name;
+
+		sprintf(fname, ".tmp_versions/%s.ns_deps", basename);
+		write_if_changed(&ns_deps_buf, fname);
+	}
+}
+
 struct ext_sym_list {
 	struct ext_sym_list *next;
 	const char *file;

@@ -2473,7 +2512,7 @@ int main(int argc, char **argv)
 	struct ext_sym_list *extsym_iter;
 	struct ext_sym_list *extsym_start = NULL;
 
-	while ((opt = getopt(argc, argv, "i:I:e:mnsST:o:awM:K:E")) != -1) {
+	while ((opt = getopt(argc, argv, "i:I:e:mnsST:o:awM:K:Ed")) != -1) {
 		switch (opt) {
 		case 'i':
 			kernel_read = optarg;

@@ -2517,6 +2556,9 @@ int main(int argc, char **argv)
 		case 'E':
 			sec_mismatch_fatal = 1;
 			break;
+		case 'd':
+			write_ns_deps = 1;
+			break;
 		default:
 			exit(1);
 		}

@@ -2545,6 +2587,12 @@ int main(int argc, char **argv)
 		check_exports(mod);
 	}
 
+	if (write_ns_deps) {
+		/* Just write namespace dependencies and exit */
+		write_ns_deps_files();
+		return 0;
+	}
+
 	err = 0;
 
 	for (mod = modules; mod; mod = mod->next) {

scripts/mod/modpost.h +2 -0

index 9626bf3e7424..92a926d375d2 100644
@@ -126,6 +126,8 @@ struct module {
 	struct buffer dev_table_buf;
 	char	     srcversion[25];
 	int is_dot_o;
+	// Required namespace dependencies
+	struct namespace_list *required_namespaces;
 	// Actual imported namespaces
 	struct namespace_list *imported_namespaces;
 };

Signature · 1 line

2.18.0.203.gfac676dfb9-goog

Modules using these symbols must now import this namesapce explicitly.

Signed-off-by Martijn Coenen <[email protected]>

---
 drivers/usb/storage/alauda.c        |  1 +
 drivers/usb/storage/cypress_atacb.c |  1 +
 drivers/usb/storage/datafab.c       |  1 +
 drivers/usb/storage/ene_ub6250.c    |  1 +
 drivers/usb/storage/freecom.c       |  1 +
 drivers/usb/storage/isd200.c        |  1 +
 drivers/usb/storage/jumpshot.c      |  1 +
 drivers/usb/storage/karma.c         |  1 +

 drivers/usb/storage/onetouch.c      |  1 +
 drivers/usb/storage/realtek_cr.c    |  1 +
 drivers/usb/storage/sddr09.c        |  1 +
 drivers/usb/storage/sddr55.c        |  1 +
 drivers/usb/storage/shuttle_usbat.c |  1 +
 drivers/usb/storage/uas.c           |  1 +
 drivers/usb/storage/usb.c           | 20 ++++++++++----------
 15 files changed, 24 insertions(+), 10 deletions(-)

This patch misses the various EXPORT_SYMBOL_GPL lines in debug.c, protocol.c, scsiglue.c, and transport.c. They should also be considered part of the USB_STORAGE namespace.

Alan Stern

As of Linux 4.17, there are more than 30000 exported symbols in the kernel. There seems to be some consensus amongst kernel devs that the export surface is too large, and hard to reason about.

Generally, these symbols fall in one of these categories:
1) Symbols actually meant for drivers
2) Symbols that are only exported because functionality is split over multiple modules, yet they really shouldn't be used by modules outside of their own subsystem
3) Symbols really only meant for in-tree use

When module developers try to upstream their code, it regularly turns out that they are using exported symbols that they really shouldn't be using. This problem is even bigger for drivers that are currently out-of-tree, which may be using many symbols that they shouldn't be using, and that break when those symbols are removed or modified.

This patch allows subsystem maintainers to partition their exported symbols into separate namespaces, and module authors to import such namespaces only when needed.

This allows subsystem maintainers to more easily limit availability of these namespaced symbols to other parts of the kernel. It can also be used to partition the set of exported symbols for documentation purposes; for example, a set of symbols that is really only used for debugging could be in a "SUBSYSTEM_DEBUG" namespace.

As of Linux 4.17, there are more than 30000 exported symbols in the kernel. There seems to be some consensus amongst kernel devs that the export surface is too large, and hard to reason about.

Generally, these symbols fall in one of these categories:
1) Symbols actually meant for drivers
2) Symbols that are only exported because functionality is split over multiple modules, yet they really shouldn't be used by modules outside of their own subsystem
3) Symbols really only meant for in-tree use

When module developers try to upstream their code, it regularly turns out that they are using exported symbols that they really shouldn't be using. This problem is even bigger for drivers that are currently out-of-tree, which may be using many symbols that they shouldn't be using, and that break when those symbols are removed or modified.

This patch allows subsystem maintainers to partition their exported symbols into separate namespaces, and module authors to import such namespaces only when needed.

This allows subsystem maintainers to more easily limit availability of these namespaced symbols to other parts of the kernel. It can also be used to partition the set of exported symbols for documentation purposes; for example, a set of symbols that is really only used for debugging could be in a "SUBSYSTEM_DEBUG" namespace.

This looks nice. I don't want to overload you with additional requests, but it might be good to think about how this can be taken further, if we want to actually convert large parts of the kernel to it.

I have two ideas:

- It would be nice to have a simple mechanism in Kconfig to put all symbols in a module into a namespace that is derived from KBUILD_MODNAME, to avoid having to annotate every symbol separately. This could be similar to how we ended up dealing with EXPORT_NO_SYMBOLS in the linux-2.4 days, with the goal of eventually having a namespace for each symbol. Similarly, we could pass a number of default namespaces through the Makefile, e.g. if we have a directory that has lots of drivers that all import the symbols from a common subsystem module.

- If this is possible to implement, it would be great to have a way to limit the globally visible symbols in a module to the ones that are explicitly exported even when a that module is built-in rather than loadable. Not sure how this is best done though. A couple of things can be done with objcopy, or possibly by reintroducing partial linking (which was removed not too long ago).

Arnd

Hi Arnd,

Quoted 4 lines · Arnd Bergmann

This looks nice. I don't want to overload you with additional requests, but it might be good to think about how this can be taken further, if we want to actually convert large parts of the kernel to it.

No worries about overloading, I'm happy with all feedback to make this better.

Quoted 12 lines

I have two ideas:

- It would be nice to have a simple mechanism in Kconfig to put all symbols in a module into a namespace that is derived from KBUILD_MODNAME, to avoid having to annotate every symbol separately. This could be similar to how we ended up dealing with EXPORT_NO_SYMBOLS in the linux-2.4 days, with the goal of eventually having a namespace for each symbol. Similarly, we could pass a number of default namespaces through the Makefile, e.g. if we have a directory that has lots of drivers that all import the symbols from a common subsystem module.

I'm hinging on two thoughts here; I really like it because it obviously reduces work and makes this easier to use. On the other hand, you now have to look in multiple places to see if a symbol is exported to a namespace/imported from a module. Perhaps it depends on how coarse-grained the namespaces end up being. Either way, I think it would be pretty easy to cook up patches for this.

Quoted 7 lines

- If this is possible to implement, it would be great to have a way to limit the globally visible symbols in a module to the ones that are explicitly exported even when a that module is built-in rather than loadable. Not sure how this is best done though. A couple of things can be done with objcopy, or possibly by reintroducing partial linking (which was removed not too long ago).

If I understand you correctly, this is orthogonal to symbol namespaces, right?

Do you think these things should be a part of these series, or can it be a follow-up?

Similar to modpost's get_next_modinfo(), introduce get_next_modinfo() in kernel/module.c to acquire any further values associated with the same modinfo tag name. That is useful for any tags that have multiple occurrences (such as 'alias'), but is in particular introduced here as part of the symbol namespaces patch series to read the (potentially) multiple namespaces a module is importing.

Reviewed-by Joel Fernandes (Google) <[email protected]>

Reviewed-by Martijn Coenen <[email protected]>

Signed-off-by Matthias Maennich <[email protected]>

This change allows growing struct kernel_symbol without wasting bytes to alignment. It also concretized the alignment of ksymtab entries if relative references are used for ksymtab entries.

struct kernel_symbol was already implicitly being aligned to the word size, except on x86_64 and m68k, where it is aligned to 16 and 2 bytes, respectively.

As far as I can tell there is no requirement for aligning struct kernel_symbol to 16 bytes on x86_64, but gcc aligns structs to their size, and the linker aligns the custom __ksymtab sections to the largest data type contained within, so setting KSYM_ALIGN to 16 was necessary to stay consistent with the code generated for non-ASM EXPORT_SYMBOL(). Now that non-ASM EXPORT_SYMBOL() explicitly aligns to word size (8), KSYM_ALIGN is no longer necessary.

In case of relative references, the alignment has been changed accordingly to not waste space when adding new struct members.

As for m68k, struct kernel_symbol is aligned to 2 bytes even though the structure itself is 8 bytes; using a 4-byte alignment shouldn't hurt.

I manually verified the output of the __ksymtab sections didn't change on x86, x86_64, arm, arm64 and m68k. As expected, the section contents didn't change, and the ELF section alignment only changed on x86_64 and m68k. Feedback from other archs more than welcome.

Co-developed-by Martijn Coenen <[email protected]>

Signed-off-by Martijn Coenen <[email protected]>

Signed-off-by Matthias Maennich <[email protected]>

The EXPORT_SYMBOL_NS() and EXPORT_SYMBOL_NS_GPL() macros can be used to export a symbol to a specific namespace. There are no _GPL_FUTURE and _UNUSED variants because these are currently unused, and I'm not sure they are necessary.

I didn't add EXPORT_SYMBOL_NS() for ASM exports; this patch sets the namespace of ASM exports to NULL by default. In case of relative references, it will be relocatable to NULL. If there's a need, this should be pretty easy to add.

A module that wants to use a symbol exported to a namespace must add a MODULE_IMPORT_NS() statement to their module code; otherwise, modpost will complain when building the module, and the kernel module loader will emit an error and fail when loading the module.

MODULE_IMPORT_NS() adds a modinfo tag 'import_ns' to the module. That tag can be observed by the modinfo command, modpost and kernel/module.c at the time of loading the module.

The ELF symbols are renamed to include the namespace with an asm label; for example, symbol 'usb_stor_suspend' in namespace USB_STORAGE becomes 'usb_stor_suspend.USB_STORAGE'. This allows modpost to do namespace checking, without having to go through all the effort of parsing ELF and relocation records just to get to the struct kernel_symbols.

On x86_64 I saw no difference in binary size (compression), but at runtime this will require a word of memory per export to hold the namespace. An alternative could be to store namespaced symbols in their own section and use a separate 'struct namespaced_kernel_symbol' for that section, at the cost of making the module loader more complex.

Co-developed-by Martijn Coenen <[email protected]>

Signed-off-by Martijn Coenen <[email protected]>

Signed-off-by Matthias Maennich <[email protected]>

Add support for symbols that are exported into namespaces. For that, extract any namespace suffix from the symbol name. In addition, emit a warning whenever a module refers to an exported symbol without explicitly importing the namespace that it is defined in. This patch consistently adds the namespace suffix to symbol names exported into Module.symvers.

Example warning emitted by modpost in case of the above violation:

WARNING: module ums-usbat uses symbol usb_stor_resume from namespace USB_STORAGE, but does not import it.

Co-developed-by Martijn Coenen <[email protected]>

Signed-off-by Martijn Coenen <[email protected]>

Reviewed-by Joel Fernandes (Google) <[email protected]>

Signed-off-by Matthias Maennich <[email protected]>

---
 scripts/mod/modpost.c | 91 +++++++++++++++++++++++++++++++++++++------
 scripts/mod/modpost.h |  7 ++++
 2 files changed, 87 insertions(+), 11 deletions(-)

If MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS is enabled (default=n), the requirement for modules to import all namespaces that are used by the module is relaxed.

Enabling this option effectively allows (invalid) modules to be loaded while only a warning is emitted.

Disabling this option keeps the enforcement at module loading time and loading is denied if the module's imports are not satisfactory.

Reviewed-by Martijn Coenen <[email protected]>

Signed-off-by Matthias Maennich <[email protected]>

---
 init/Kconfig    | 14 ++++++++++++++
 kernel/module.c | 11 +++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

init/Kconfig +0 -0

index bd7d650d4a99..b3373334cdf1 100644

init/Kconfig +11 -0

+	help
+	  Symbols exported with EXPORT_SYMBOL_NS*() are considered exported in
+	  a namespace. A module that makes use of a symbol exported with such a
+	  namespace is required to import the namespace via MODULE_IMPORT_NS().
+	  This option relaxes this requirement when loading a module. While
+	  technically there is no reason to enforce correct namespace imports,
+	  it creates consistency between symbols defining namespaces and users
+	  importing namespaces they make use of.
+
+	  If unsure, say N.
+
 config TRIM_UNUSED_KSYMS
 	bool "Trim unused exported kernel symbols"
 	depends on MODULES && !UNUSED_SYMBOLS

kernel/module.c +0 -2

index 57e8253f2251..7c934aaae2d3 100644
@@ -1408,9 +1408,16 @@ static int verify_namespace_is_imported(const struct load_info *info,
 			imported_namespace = get_next_modinfo(
 				info, "import_ns", imported_namespace);
 		}
-		pr_err("%s: module uses symbol (%s) from namespace %s, but does not import it.\n",
-		       mod->name, kernel_symbol_name(sym), namespace);

If MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS is enabled (default=n), the requirement for modules to import all namespaces that are used by the module is relaxed.

Enabling this option effectively allows (invalid) modules to be loaded while only a warning is emitted.

Disabling this option keeps the enforcement at module loading time and loading is denied if the module's imports are not satisfactory.

Reviewed-by Martijn Coenen <[email protected]>

Signed-off-by Matthias Maennich <[email protected]>

---
 init/Kconfig    | 14 ++++++++++++++
 kernel/module.c | 11 +++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

init/Kconfig +8 -0

index bd7d650d4a99..b3373334cdf1 100644
@@ -2119,6 +2119,20 @@ config MODULE_COMPRESS_XZ
 
 endchoice
 
+config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS
+       bool "Allow loading of modules with missing namespace imports"
+       default n
+       help
+         Symbols exported with EXPORT_SYMBOL_NS*() are considered exported in
+         a namespace. A module that makes use of a symbol exported with such a
+         namespace is required to import the namespace via MODULE_IMPORT_NS().
+         This option relaxes this requirement when loading a module.
 

While

Quoted 34 lines · Matthias Maennich' via kernel-team

If MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS is enabled (default=n), the requirement for modules to import all namespaces that are used by the module is relaxed.

Enabling this option effectively allows (invalid) modules to be loaded while only a warning is emitted.

Disabling this option keeps the enforcement at module loading time and loading is denied if the module's imports are not satisfactory.

Reviewed-by Martijn Coenen <[email protected]>

Signed-off-by Matthias Maennich <[email protected]>

---
 init/Kconfig    | 14 ++++++++++++++
 kernel/module.c | 11 +++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

init/Kconfig +8 -0

index bd7d650d4a99..b3373334cdf1 100644
@@ -2119,6 +2119,20 @@ config MODULE_COMPRESS_XZ
 
 endchoice
 
+config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS
+       bool "Allow loading of modules with missing namespace imports"
+       default n
+       help
+         Symbols exported with EXPORT_SYMBOL_NS*() are considered exported in
+         a namespace. A module that makes use of a symbol exported with such a
+         namespace is required to import the namespace via MODULE_IMPORT_NS().
+         This option relaxes this requirement when loading a module.

init/Kconfig config MODULE_COMPRESS_XZ · line 2132

Quoted 4 lines

While

init/Kconfig +3 -0

+         technically there is no reason to enforce correct namespace imports,
+         it creates consistency between symbols defining namespaces and users
+         importing namespaces they make use of.

I'm confused by this sentence. It sounds like it's the opposite of what the config is doing? Can you please reword it for clarify?