linux-kernel - Re: [PATCH] block2mtd lockdep_init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200801081147.00701.rusty@rustcorp.com.au>
Date:	Tue, 8 Jan 2008 11:47:00 +1100
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Erez Zadok <ezk@...sunysb.edu>,
	Jörn Engel <joern@...fs.org>,
	dwmw2@...radead.org, linux-mtd@...ts.infradead.org,
	mingo@...hat.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] block2mtd lockdep_init_map warning

On Monday 07 January 2008 21:05:26 Peter Zijlstra wrote:
> On Sun, 2008-01-06 at 14:11 -0500, Erez Zadok wrote:
> > > Ingo, Peter, does either of you actually care about this problem?  In
> > > the last round when I debugged this problem there was a notable lack of
> > > reaction from either of you.
> >
> > The problem appears to be an interaction of two components--module
> > loading and lockdep--that's perhaps why it wasn't given enough attention.
>
> Would something like this work for people?

Hi Peter,

    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?

Thanks,
Rusty.

diff -r 68fd1b22db89 kernel/module.c
--- a/kernel/module.c	Mon Jan 07 18:59:50 2008 +1100
+++ b/kernel/module.c	Tue Jan 08 11:46:11 2008 +1100
@@ -2043,6 +2043,11 @@ static struct module *load_module(void _
 		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 *)
@@ -2051,7 +2056,7 @@ static struct module *load_module(void _
 			 / sizeof(struct kernel_param),
 			 NULL);
 	if (err < 0)
-		goto arch_cleanup;
+		goto unlink;
 
 	err = mod_sysfs_setup(mod,
 			      (struct kernel_param *)
@@ -2059,7 +2064,7 @@ static struct module *load_module(void _
 			      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);
 
@@ -2074,7 +2079,8 @@ static struct module *load_module(void _
 	/* Done! */
 	return mod;
 
- arch_cleanup:
+ unlink:
+	stop_machine_run(__unlink_module, mod, NR_CPUS);
 	module_arch_cleanup(mod);
  cleanup:
 	module_unload_free(mod);
@@ -2130,10 +2136,6 @@ sys_init_module(void __user *umod,
 		mutex_unlock(&module_mutex);
 		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);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/