Messages in this thread Patch in this message |  | | Date | Sun, 27 Aug 2000 03:17:05 +1100 | From | Andrew Morton <> | Subject | [patch] kernel/module.c |
| |
There are two bugs in sys_init_module():
- The loop which says it does "Update module references" does the updating at the same time as performing some sanity checks. If the sanity checks fail it bales out, leaving the module reference tree in an inconsistent state.
I've split the sanity checking phase and the updating phase apart.
- When the actual module text is copied down from user space this overwrites the module's name, which was installed at (mod + 1) by sys_create_module(). If something goes wrong after this copy, the module no longer has a name and modutils can no longer get at it to clean up.
I took a copy of the name and restored it in the error path. Used a nasty cast to overcome the constness of module.name.
- I added this comment above the kernel_module initialiser:
+/* + * Don't turn this initialiser into "name: value," form. + * gcc-2.7.2.3 lays it out incorrectly (andrewm@uow.edu.au) + */ :-(
This script:
#!/bin/sh
modbash() { while true do rmmod $1 insmod $1 done }
modbash vfat & modbash fat
demonstrates the problem. It turns the kernel module system into a smoking ruin in about 0.25 seconds. Uniprocessor.
The problem is that insmod is racing against rmmod. insmod reads the module symbol table and prepares a module for insertion. But when the insertion occurs, things have changed, the sanity checks do the wrong thing as described above and everything goes pear-shaped.
Kernel 2.2 has the same bugs.
--- linux-2.4.0-test7/kernel/module.c Tue Jul 11 22:21:17 2000 +++ linux-akpm/kernel/module.c Sat Aug 26 23:13:11 2000 @@ -6,6 +6,7 @@ #include <linux/smp_lock.h> #include <asm/pgalloc.h> #include <linux/init.h> +#include <linux/slab.h> /* * Originally by Anonymous (as far as I know...) @@ -26,6 +27,10 @@ extern const struct exception_table_entry __start___ex_table[]; extern const struct exception_table_entry __stop___ex_table[]; +/* + * Don't turn this initialiser into "name: value," form. + * gcc-2.7.2.3 lays it out incorrectly (andrewm@uow.edu.au) + */ static struct module kernel_module = { sizeof(struct module), /* size_of_struct */ @@ -160,7 +165,7 @@ sys_init_module(const char *name_user, struct module *mod_user) { struct module mod_tmp, *mod; - char *name, *n_name; + char *name, *n_name, *name_tmp; long namelen, n_namelen, i, error; unsigned long mod_user_size; struct module_ref *dep; @@ -194,6 +199,12 @@ /* Hold the current contents while we play with the user's idea of righteousness. */ mod_tmp = *mod; + name_tmp = kmalloc(strlen(mod->name) + 1, GFP_KERNEL); /* Where's kstrdup()? */ + if (name_tmp == NULL) { + error = -ENOMEM; + goto err1; + } + strcpy(name_tmp, mod->name); error = copy_from_user(mod, mod_user, sizeof(struct module)); if (error) { @@ -290,9 +301,10 @@ to make the I and D caches consistent. */ flush_icache_range((unsigned long)mod, (unsigned long)mod + mod->size); - /* Update module references. */ mod->next = mod_tmp.next; mod->refs = NULL; + + /* Sanity check the module's dependents */ for (i = 0, dep = mod->deps; i < mod->ndeps; ++i, ++dep) { struct module *o, *d = dep->dep; @@ -303,14 +315,21 @@ goto err3; } - for (o = module_list; o != &kernel_module; o = o->next) - if (o == d) goto found_dep; + /* Scan the current modules for this dependency */ + for (o = module_list; o != &kernel_module && o != d; o = o->next) + ; - printk(KERN_ERR "init_module: found dependency that is " + if (o != d) { + printk(KERN_ERR "init_module: found dependency that is " "(no longer?) a module.\n"); - goto err3; - - found_dep: + goto err3; + } + } + + /* Update module references. */ + for (i = 0, dep = mod->deps; i < mod->ndeps; ++i, ++dep) { + struct module *d = dep->dep; + dep->ref = mod; dep->next_ref = d->refs; d->refs = dep; @@ -344,6 +363,8 @@ put_mod_name(n_name); err2: *mod = mod_tmp; + strcpy((char *)mod->name, name_tmp); /* We know there is room for this */ + kfree(name_tmp); err1: put_mod_name(name); err0: - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/
|  |