| 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
| ||
|
Date: Tue, 1 Jun 2010 16:24:31 -0700
From: Brandon Philips <brandon@...p.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Rusty Russell <rusty@...tcorp.com.au>,
Andrew Morton <akpm@...ux-foundation.org>,
"Rafael J. Wysocki" <rjw@...k.pl>,
LKML <linux-kernel@...r.kernel.org>,
Jon Masters <jonathan@...masters.org>,
Tejun Heo <htejun@...il.com>,
Masami Hiramatsu <mhiramat@...hat.com>,
Kay Sievers <kay.sievers@...y.org>
Subject: Re: [PATCH 2/2] module: fix bne2 "gave up waiting for init of
module libcrc32c"
On 10:53 Tue 01 Jun 2010, Linus Torvalds wrote:
> On Tue, 1 Jun 2010, Linus Torvalds wrote:
> > So I do agree that your later two-patch series is the way to go.
>
> Oh, btw, did we ever get that version tested by the only person who seems
> to see the problem? Brandon?
Sorry for the delay. I broke the fuse on the remote power switch I was
using.
This is the subset of patches that everyone agreed, right?
git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux-2.6 modules
Otherwise what patches should I be applying to 2.6.35-rc1?
When I tested a Kernel with Rusty's modules branch pulled onto
2.6.35-rc1 I got duplicate sysfs path errors:
[ 8.985303] WARNING: at fs/sysfs/dir.c:451 sysfs_add_one+0xb2/0xd0()
[ 8.998161] Hardware name: Dinar
[ 9.004734] sysfs: cannot create duplicate filename '/module/libcrc32c'
[ 9.018068] Modules linked in: libcrc32c(+) button ohci_hcd
ehci_hcd usbcore sd_mod crc_t10dif edd ext3 mbcache jbd fan thermal
processor thermal_sys hwmon ide_pci_generic atiixp ide_core
ata_generic ahci libahci pata_atiixp libata scsi_mod
[ 9.062682] Pid: 1588, comm: modprobe Not tainted 2.6.35-rc1-0.7-default #3
[ 9.076711] Call Trace:
[ 9.081731] [<ffffffff8116a162>] ? sysfs_add_one+0xb2/0xd0
[ 9.092988] [<ffffffff8104f6aa>] warn_slowpath_common+0x7a/0xd0
[ 9.105115] [<ffffffff8104f7a1>] warn_slowpath_fmt+0x41/0x50
[ 9.116718] [<ffffffff8116a162>] sysfs_add_one+0xb2/0xd0
[ 9.127628] [<ffffffff8116ace5>] create_dir+0x75/0xc0
[ 9.138015] [<ffffffff8116adc6>] sysfs_create_dir+0x96/0xc0
[ 9.149447] [<ffffffff811c5297>] kobject_add_internal+0xe7/0x250
[ 9.161744] [<ffffffff811c5518>] kobject_add_varg+0x38/0x60
[ 9.173173] [<ffffffff811c5593>] kobject_init_and_add+0x53/0x70
[ 9.185295] [<ffffffff811c5160>] ? kset_find_obj+0x40/0x90
[ 9.196555] [<ffffffff8107f193>] mod_sysfs_init+0x83/0xf0
[ 9.207639] [<ffffffff810807ec>] load_module+0xbec/0x1e10
[ 9.218723] [<ffffffff810d7467>] ? handle_mm_fault+0x227/0xaf0
[ 9.230672] [<ffffffff810da47c>] ? vma_link+0xac/0x110
[ 9.241236] [<ffffffff81385168>] ? do_page_fault+0x228/0x410
[ 9.252839] [<ffffffff810dcc19>] ? do_mmap_pgoff+0x359/0x380
[ 9.264442] [<ffffffff81081a6a>] sys_init_module+0x5a/0x220
[ 9.275873] [<ffffffff81002ec2>] system_call_fastpath+0x16/0x1b
[ 9.287993] ---[ end trace 2a73785cc061234f ]---
[ 9.297350] kobject_add_internal failed for libcrc32c with -EEXIST,
don't try to register things with the same name in the same directory.
To fix this we need to make sure that we only have one copy of a module
going through load_module at a time. Patch suggestion follows which
boots without errors. I am sure there is a better way to do what it does
;)
Full dmesg:
http://ifup.org/~philips/rusty-module-branch.dmesg
Cheers,
Brandon
>From 35c6bb7c9a48b3ce53bc4b51871e6bed6e09c80a Mon Sep 17 00:00:00 2001
From: Brandon Philips <bphilips@...e.de>
Date: Wed, 2 Jun 2010 01:13:54 +0200
Subject: [PATCH] module: track module names currently loading
After "make locking more fine-grained" multiple instances of the same module
can reach mod_sysfs_init() and spew -EEXIST from kobject_add_internal().
Track the modules that are currently in load_module() but are not in the
modules list yet so we can kick out duplicate modules early.
NOTE: I didn't use mod->list to track the loading modules because I
couldn't figure out how to unwind from an error in a nice way.
Signed-off-by: Brandon Philips <bphilips@...e.de>
---
kernel/module.c | 45 +++++++++++++++++++++++++++++++++++++++++----
1 file changed, 41 insertions(+), 4 deletions(-)
Index: linux-2.6/kernel/module.c
===================================================================
--- linux-2.6.orig/kernel/module.c
+++ linux-2.6/kernel/module.c
@@ -77,6 +77,7 @@
DEFINE_MUTEX(module_mutex);
EXPORT_SYMBOL_GPL(module_mutex);
static LIST_HEAD(modules);
+static LIST_HEAD(modules_loading);
#ifdef CONFIG_KGDB_KDB
struct list_head *kdb_modules = &modules; /* kdb needs the list of modules */
#endif /* CONFIG_KGDB_KDB */
@@ -2055,6 +2056,23 @@ static inline void kmemleak_load_module(
}
#endif
+/* Track module names running through load_module but not in modules list */
+struct module_name {
+ struct list_head list;
+ char name[MODULE_NAME_LEN];
+};
+
+static struct module_name *find_module_loading(const char *name)
+{
+ struct module_name *mod_name;
+
+ list_for_each_entry(mod_name, &modules_loading, list) {
+ if (strcmp(mod_name->name, name) == 0)
+ return mod_name;
+ }
+ return NULL;
+}
+
/* Allocate and load the module: note that size of section 0 is always
zero, and we rely on this for optional sections. */
static noinline struct module *load_module(void __user *umod,
@@ -2074,6 +2092,7 @@ static noinline struct module *load_modu
void *ptr = NULL; /* Stops spurious gcc warning */
unsigned long symoffs, stroffs, *strmap;
void __percpu *percpu;
+ struct module_name *mod_name;
mm_segment_t old_fs;
@@ -2198,24 +2217,36 @@ static noinline struct module *load_modu
goto free_mod;
}
- if (find_module(mod->name)) {
- err = -EEXIST;
+ mod_name = kzalloc(sizeof(*mod_name), GFP_KERNEL);
+ if (!mod_name) {
+ err = -ENOMEM;
goto free_mod;
}
+ strcpy(mod_name->name, mod->name);
+ INIT_LIST_HEAD(&mod_name->list);
+
+ mutex_lock(&module_mutex);
+ if (find_module(mod->name) || find_module_loading(mod->name)) {
+ mutex_unlock(&module_mutex);
+ err = -EEXIST;
+ goto free_mod_name;
+ }
+ list_add(&mod_name->list, &modules_loading);
+ mutex_unlock(&module_mutex);
mod->state = MODULE_STATE_COMING;
/* Allow arches to frob section contents and sizes. */
err = module_frob_arch_sections(hdr, sechdrs, secstrings, mod);
if (err < 0)
- goto free_mod;
+ goto del_loading;
if (pcpuindex) {
/* We have a special allocation for this section. */
err = percpu_modalloc(mod, sechdrs[pcpuindex].sh_size,
sechdrs[pcpuindex].sh_addralign);
if (err)
- goto free_mod;
+ goto del_loading;
sechdrs[pcpuindex].sh_flags &= ~(unsigned long)SHF_ALLOC;
}
/* Keep this around for failure path. */
@@ -2486,6 +2517,8 @@ static noinline struct module *load_modu
* The mutex protects against concurrent writers.
*/
mutex_lock(&module_mutex);
+ list_del(&mod_name->list);
+ kfree(mod_name);
list_add_rcu(&mod->list, &modules);
mutex_unlock(&module_mutex);
@@ -2530,6 +2563,10 @@ static noinline struct module *load_modu
/* mod will be freed with core. Don't access it beyond this line! */
free_percpu:
free_percpu(percpu);
+ del_loading:
+ list_del(&mod_name->list);
+ free_mod_name:
+ kfree(mod_name);
free_mod:
kfree(args);
kfree(strmap);
--
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/
Powered by blists - more mailing lists