[PATCH] block2mtd lockdep_init_map warning

Jörn Engel joern at logfs.org
Wed Jan 16 16:20:54 EST 2008


On Tue, 8 January 2008 11:47:00 +1100, Rusty Russell wrote:
> 
>     There's nothing wrong with this patch, but I think it papers over a more
> general problem: we enter the module (to parse args) while it's not in the
> module list.  This also means we won't get a nice oops if it crashes.
> 
>     This is untested, but does it solve it for you?

Not sure about lockdep, but it makes all the difference for an oops,
forced by explicit BUG() in block2mtd.

Before:
kernel BUG at drivers/mtd/devices/block2mtd.c:382!
invalid opcode: 0000 [#1]
Modules linked in:
CPU:    0
EIP:    0060:[<e0818003>]    Not tainted VLI
EFLAGS: 00000246   (2.6.22 #38)
EIP is at 0xe0818003
eax: dfe8c7da   ebx: dfe8c7d0   ecx: e08185a8   edx: e08185a8
esi: e0818b1c   edi: dfe8c7da   ebp: df95de78   esp: df95de78
ds: 007b   es: 007b   fs: 0000  gs: 0033  ss: 0068
Process insmod (pid: 1076, ti=df95c000 task=df903080 task.ti=df95c000)
Stack: df95dec4 c0125576 00000000 df95de90 fffffff8 e0815932 00000006 00000000 
       e08185a8 e0818b4c 00000283 00000000 0000000b e0818b1c 00000000 dfe8c7dc 
       00000014 e0815b48 00000014 df95dfb0 c01314e4 00000001 00000000 df95df24 
Call Trace:
 [<c0103248>] show_trace_log_lvl+0x1a/0x2f
 [<c01032f8>] show_stack_log_lvl+0x9b/0xa3
 [<c01034be>] show_registers+0x1be/0x290
 [<c010366f>] die+0xdf/0x1a1
 [<c01037ba>] do_trap+0x89/0xa2
 [<c0103b25>] do_invalid_op+0x88/0x92
 [<c0384e82>] error_code+0x6a/0x70
 [<c0125576>] parse_args+0x120/0x1f6
 [<c01314e4>] sys_init_module+0xf76/0x1297
 [<c01023ee>] syscall_call+0x7/0xb
 =======================
Code: <0f> 0b eb fe 55 85 c0 89 e5 53 89 c3 74 31 8b 40 28 e8 ee 3e 93 df 
EIP: [<e0818003>] 0xe0818003 SS:ESP 0068:df95de78

After:
kernel BUG at drivers/mtd/devices/block2mtd.c:382!
invalid opcode: 0000 [#1]
Modules linked in: block2mtd
CPU:    0
EIP:    0060:[<f8820003>]    Not tainted VLI
EFLAGS: 00000246   (2.6.22 #39)
EIP is at block2mtd_setup+0x3/0x7 [block2mtd]
eax: f7da387a   ebx: f7da3870   ecx: f88205a8   edx: f88205a8
esi: f8820b1c   edi: f7da387a   ebp: f788be78   esp: f788be78
ds: 007b   es: 007b   fs: 0000  gs: 0033  ss: 0068
Process insmod (pid: 1072, ti=f788a000 task=c1b22b00 task.ti=f788a000)
Stack: f788bec4 c0125576 00000000 f788be90 fffffff8 f881d932 00000006 00000000 
       f88205a8 f8820b4c 00000283 00000000 0000000b f8820b1c 00000000 f7da387c 
       00000014 f881db48 00000014 f788bfb0 c01314f1 00000001 00000000 f788bf24 
Call Trace:
 [<c0103248>] show_trace_log_lvl+0x1a/0x2f
 [<c01032f8>] show_stack_log_lvl+0x9b/0xa3
 [<c01034be>] show_registers+0x1be/0x290
 [<c010366f>] die+0xdf/0x1a1
 [<c01037ba>] do_trap+0x89/0xa2
 [<c0103b25>] do_invalid_op+0x88/0x92
 [<c0384e92>] error_code+0x6a/0x70
 [<c0125576>] parse_args+0x120/0x1f6
 [<c01314f1>] sys_init_module+0xf83/0x12a4
 [<c01023ee>] syscall_call+0x7/0xb
 =======================
Code: <0f> 0b eb fe 55 85 c0 89 e5 53 89 c3 74 31 8b 40 28 e8 fa be 92 c7 
EIP: [<f8820003>] block2mtd_setup+0x3/0x7 [block2mtd] SS:ESP 0068:f788be78


Patch didn't compile due to function ordering.  Here is an updated version.

Acked-and-tested-by: Joern Engel <joern at lazybastard.org>

diff --git a/kernel/module.c b/kernel/module.c
index c2e3e2e..0397b1e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1645,6 +1645,17 @@ static inline void add_kallsyms(struct module *mod,
 }
 #endif /* CONFIG_KALLSYMS */
 
+/*
+ * link the module with the whole machine is stopped with interrupts off
+ * - this defends against kallsyms not taking locks
+ */
+static int __link_module(void *_mod)
+{
+	struct module *mod = _mod;
+	list_add(&mod->list, &modules);
+	return 0;
+}
+
 /* Allocate and load the module: note that size of section 0 is always
    zero, and we rely on this for optional sections. */
 static struct module *load_module(void __user *umod,
@@ -2023,6 +2034,11 @@ static struct module *load_module(void __user *umod,
 		printk(KERN_WARNING "%s: Ignoring obsolete parameters\n",
 		       mod->name);
 
+	/* Now sew it into the lists so we can get lockdep and oops
+         * info during argument parsing.  Noone should access us, since
+         * strong_try_module_get() will fail. */
+	stop_machine_run(__link_module, mod, NR_CPUS);
+
 	/* Size of section 0 is 0, so this works well if no params */
 	err = parse_args(mod->name, mod->args,
 			 (struct kernel_param *)
@@ -2031,7 +2047,7 @@ static struct module *load_module(void __user *umod,
 			 / sizeof(struct kernel_param),
 			 NULL);
 	if (err < 0)
-		goto arch_cleanup;
+		goto unlink;
 
 	err = mod_sysfs_setup(mod,
 			      (struct kernel_param *)
@@ -2039,7 +2055,7 @@ static struct module *load_module(void __user *umod,
 			      sechdrs[setupindex].sh_size
 			      / sizeof(struct kernel_param));
 	if (err < 0)
-		goto arch_cleanup;
+		goto unlink;
 	add_sect_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
 	add_notes_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
 
@@ -2054,7 +2070,8 @@ static struct module *load_module(void __user *umod,
 	/* Done! */
 	return mod;
 
- arch_cleanup:
+ unlink:
+	stop_machine_run(__unlink_module, mod, NR_CPUS);
 	module_arch_cleanup(mod);
  cleanup:
 	module_unload_free(mod);
@@ -2076,17 +2093,6 @@ static struct module *load_module(void __user *umod,
 	goto free_hdr;
 }
 
-/*
- * link the module with the whole machine is stopped with interrupts off
- * - this defends against kallsyms not taking locks
- */
-static int __link_module(void *_mod)
-{
-	struct module *mod = _mod;
-	list_add(&mod->list, &modules);
-	return 0;
-}
-
 /* This is where the real work happens */
 asmlinkage long
 sys_init_module(void __user *umod,
@@ -2111,10 +2117,6 @@ sys_init_module(void __user *umod,
 		return PTR_ERR(mod);
 	}
 
-	/* Now sew it into the lists.  They won't access us, since
-           strong_try_module_get() will fail. */
-	stop_machine_run(__link_module, mod, NR_CPUS);
-
 	/* Drop lock so they can recurse */
 	mutex_unlock(&module_mutex);
 



More information about the linux-mtd mailing list