tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Enabling built-in modules earlier in init



On Wed, 16 Jun 2010, Antti Kantee wrote:

On Wed Jun 16 2010 at 06:31:59 -0700, Paul Goyette wrote:
The attached diffs add a new mod_disabled member to the module_t
structure, and set the value to false in each place that a new entry is
created.  (Since all of the allocations of module_t structures are done
with kmem_zalloc() I could probably avoid the explicit setting of the
value to false.)

The value is set to true whenever a module is removed from active duty
and returned to the module_builtin list.  (I specifically did NOT mark a
module disabled if its modcmd(INIT) failed, under the assumption that it
might succeed in a later retry.)

Keeping the same security use case in mind, it would be better that after
full module bootstrap (i.e. MODULE_CLASS_ANY) all builtin modules would
be either initialized or disabled.  Otherwise, if we assume that init
may later succeed for whatever reason, an operator that checks a module
with a security problem is not activated may be surprised to later find
out that the same module has now been autoenabled.

Good point.

The attached diffs add one more routine, module_init3() which gets called from init_main() right after module_class_init(MODULE_CLASS_ANY). module_init3() walks the list of builtin modules that have not already been init'd and marks them disabled.

Tested briefly on my home systems and appears to work.

Any objections to committing this?



-------------------------------------------------------------------------
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:       |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com    |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer |                          | pgoyette at netbsd.org  |
-------------------------------------------------------------------------
Index: sys/sys/module.h
===================================================================
RCS file: /cvsroot/src/sys/sys/module.h,v
retrieving revision 1.23
diff -u -p -r1.23 module.h
--- sys/sys/module.h    24 May 2010 03:50:25 -0000      1.23
+++ sys/sys/module.h    16 Jun 2010 22:35:05 -0000
@@ -90,6 +90,7 @@ typedef struct module {
        time_t                  mod_autotime;
        void                    *mod_ctf;
        u_int                   mod_fbtentries; /* DTrace FBT entrie count */
+       bool                    mod_disabled;
 } module_t;
 
 /*
@@ -121,6 +122,7 @@ extern u_int                module_gen;
 
 void   module_init(void);
 void   module_init2(void);
+void   module_init3(void);
 void   module_init_md(void);
 void   module_init_class(modclass_t);
 int    module_prime(void *, size_t);
Index: sys/kern/kern_module.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_module.c,v
retrieving revision 1.69
diff -u -p -r1.69 kern_module.c
--- sys/kern/kern_module.c      26 May 2010 23:53:21 -0000      1.69
+++ sys/kern/kern_module.c      16 Jun 2010 22:35:05 -0000
@@ -195,6 +195,7 @@ module_builtin_add(modinfo_t *const *mip
                modp[i] = kmem_zalloc(sizeof(*modp[i]), KM_SLEEP);
                modp[i]->mod_info = mip[i+mipskip];
                modp[i]->mod_source = MODULE_SOURCE_KERNEL;
+               modp[i]->mod_disabled = false;
        }
        mutex_enter(&module_lock);
 
@@ -350,6 +351,23 @@ module_init2(void)
                panic("module_init: %d", error);
 }
 
+/*
+ * module_init3:
+ *
+ * Disable any built-in modules that have not yet been initialized.
+ */
+void
+module_init3(void)
+{
+       module_t *mod;
+
+       mutex_enter(&module_lock);
+       TAILQ_FOREACH(mod, &module_builtins, mod_chain) {
+               mod->mod_disabled = true;
+       }
+       mutex_exit(&module_lock);
+}
+
 static struct sysctllog *module_sysctllog;
 
 static void
@@ -416,6 +434,7 @@ module_init_class(modclass_t class)
                         * init.
                         */
                        if (module_do_builtin(mi->mi_name, NULL) != 0) {
+                               mod->mod_disabled = true;
                                TAILQ_REMOVE(&module_builtins, mod, mod_chain);
                                TAILQ_INSERT_TAIL(&bi_fail, mod, mod_chain);
                        }
@@ -794,7 +813,7 @@ module_do_load(const char *name, bool is
                }
        }
        if (mod) {
-               if ((flags & MODCTL_LOAD_FORCE) == 0) {
+               if (mod->mod_disabled && (flags & MODCTL_LOAD_FORCE) == 0) {
                        if (!autoload) {
                                module_error("use -f to reinstate "
                                    "builtin module \"%s\"", name);
@@ -854,6 +873,7 @@ module_do_load(const char *name, bool is
                        return error;
                }
                mod->mod_source = MODULE_SOURCE_FILESYS;
+               mod->mod_disabled = false;
                TAILQ_INSERT_TAIL(&pending, mod, mod_chain);
 
                error = module_fetch_info(mod);
@@ -1086,6 +1106,7 @@ module_do_unload(const char *name)
        }
        if (mod->mod_source == MODULE_SOURCE_KERNEL) {
                mod->mod_nrequired = 0; /* will be re-parsed */
+               mod->mod_disabled = true;
                TAILQ_INSERT_TAIL(&module_builtins, mod, mod_chain);
                module_builtinlist++;
        } else {
@@ -1129,6 +1150,7 @@ module_prime(void *base, size_t size)
        }
 
        TAILQ_INSERT_TAIL(&module_bootlist, mod, mod_chain);
+       mod->mod_disabled = false;
 
        return 0;
 }
Index: sys/kern/init_main.c
===================================================================
RCS file: /cvsroot/src/sys/kern/init_main.c,v
retrieving revision 1.420
diff -u -p -r1.420 init_main.c
--- sys/kern/init_main.c        10 Jun 2010 20:54:53 -0000      1.420
+++ sys/kern/init_main.c        16 Jun 2010 22:35:05 -0000
@@ -594,9 +594,11 @@ main(void)
 
        /*
         * Load any remaining builtin modules, and hand back temporary
-        * storage to the VM system.
+        * storage to the VM system.  Then mark any remaining un-init'ed
+        * built-in modules as disabled to avoid later surprises.
         */
        module_init_class(MODULE_CLASS_ANY);
+       module_init3();
 
        /*
         * Finalize configuration now that all real devices have been


Home | Main Index | Thread Index | Old Index