introduce MODULE_USE_UNEXPORTED()

may-be-patch? Oleg Nesterov 2 participants 2 messages Jul 19, 2011 – Jul 21, 2011 Module support

lore

Share & tools

lore

Oleg Nesterov

author

2011-07-19 18:48 UTC #0 lore raw

Hi.

Quoted 1 line

From time to time I need to write a module for debugging purposes,

and it is a bit annoying I can't simply use, say, put_task_struct().

Can't we do something like the simple patch below? Of course, it is very unsafe to use kallsyms_lookup_name() by many reasons. But it looks handy for the testing/debugging.

Yes, I am not saying this feature is terribly useful, so I agree in advance with the "go away, we don't need this hack" nack.

Still, what do you think?

Oleg.

 include/linux/module.h |    2 ++
 include/linux/kernel.h |    1 +
 kernel/module.c        |   13 +++++++++++++
 kernel/panic.c         |    2 ++
 4 files changed, 18 insertions(+)

include/linux/module.h renamed from include/linux/module.h~MODSYM +2 -0

@@ -139,6 +139,8 @@ extern struct module __this_module;
  */
 #define MODULE_LICENSE(_license) MODULE_INFO(license, _license)
 
+#define MODULE_USE_UNEXPORTED()	MODULE_INFO(use_unexported, "")
+
 /*
  * Author(s), use "Name <email>" or just "Name", for multiple
  * authors use multiple MODULE_AUTHOR() statements/lines.

include/linux/kernel.h renamed from include/linux/kernel.h~MODSYM +1 -0

@@ -361,6 +361,7 @@ extern enum system_states {
 #define TAINT_WARN			9
 #define TAINT_CRAP			10
 #define TAINT_FIRMWARE_WORKAROUND	11
+#define TAINT_USE_UNEXPORTED		12
 
 extern const char hex_asc[];
 #define hex_asc_lo(x)	hex_asc[((x) & 0x0f)]

kernel/module.c renamed from kernel/module.c~MODSYM +13 -0

@@ -1791,6 +1791,8 @@ static int verify_export_symbols(struct 
 	return 0;
 }
 
+static char *get_modinfo(struct load_info *info, const char *tag);
+
 /* Change all symbols so that st_value encodes the pointer directly. */
 static int simplify_symbols(struct module *mod, const struct load_info *info)
 {

@@ -1832,6 +1834,17 @@ static int simplify_symbols(struct modul
 			if (!ksym && ELF_ST_BIND(sym[i].st_info) == STB_WEAK)
 				break;
 
+			if (!ksym && get_modinfo(info, "use_unexported")) {
+				unsigned long addr = kallsyms_lookup_name(name);
+				if (addr) {
+					sym[i].st_value = addr;
+					if (!test_taint(TAINT_USE_UNEXPORTED))
+						printk(KERN_WARNING "bla bla bla\n");
+					add_taint_module(mod, TAINT_USE_UNEXPORTED);
+					break;
+				}
+			}
+
 			printk(KERN_WARNING "%s: Unknown symbol %s (err %li)\n",
 			       mod->name, name, PTR_ERR(ksym));
 			ret = PTR_ERR(ksym) ?: -ENOENT;

kernel/panic.c renamed from kernel/panic.c~MODSYM +2 -0

@@ -175,6 +175,7 @@ static const struct tnt tnts[] = {
 	{ TAINT_WARN,			'W', ' ' },
 	{ TAINT_CRAP,			'C', ' ' },
 	{ TAINT_FIRMWARE_WORKAROUND,	'I', ' ' },
+	{ TAINT_USE_UNEXPORTED,		'X', ' ' },
 };
 
 /**

@@ -192,6 +193,7 @@ static const struct tnt tnts[] = {
  *  'W' - Taint on warning.
  *  'C' - modules from drivers/staging are loaded.
  *  'I' - Working around severe firmware bug.
+ *  'X' - Module uses unexported symbol.
  *
  *	The string is overwritten by the next call to print_tainted().
  */

Rusty Russell

2011-07-21 09:33 UTC #1 lore raw

Quoted 9 lines · Oleg Nesterov

Hi.
Quoted 1 line
From time to time I need to write a module for debugging purposes,
and it is a bit annoying I can't simply use, say, put_task_struct().
Can't we do something like the simple patch below? Of course, it is very unsafe to use kallsyms_lookup_name() by many reasons. But it looks handy for the testing/debugging.

Quoted 6 lines

Yes, I am not saying this feature is terribly useful, so I agree in advance with the "go away, we don't need this hack" nack.
Still, what do you think?
Oleg.

I hate it, but I've wanted it too...

Can we bury it under a CONFIG_DEBUG_KERNEL option? I don't want distributions turning this on. And add a GPL license check.

Alternative is to have a "void *khacker_lookup_sym(const char *)" which does this at runtime, but that's less convenient to use than this.

Thanks,
Rusty.