[patch] kernel/module.c

From: Andrew Morton (andrewm@uow.edu.au)
Date: Sat Aug 26 2000 - 11:17:05 EST


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/



This archive was generated by hypermail 2b29 : Thu Aug 31 2000 - 21:00:17 EST