Re: [patch 5/12] lsm stacking v0.2: actual stacker module

From: Tony Jones
Date: Sun Jul 03 2005 - 22:24:04 EST


Hey Serge,

I don't think your symbol_get() is doing what you think it is ;-)

> + * Add the stacked module (as specified by name and ops).
> + * If the module is not compiled in, the symbol_get at the end will
> + * prevent the the module from being unloaded.
> +*/
> +static int stacker_register (const char *name, struct security_operations *ops)
> +{
...
> + symbol_get(ops);
> +
> +out:
> + spin_unlock(&stacker_lock);
> + return ret;
> +}


Seemed useful to be able to view which modules had been unloaded.
Easier to maintain them on their own list than to compute the difference
of <stacked_modules> and <all_modules>. Patch attached, not sure if you
are cool with reusing the 'unload' file.

> +static struct stacker_attribute stacker_attr_unload = {
> + .attr = {.name = "unload", .mode = S_IFREG | S_IRUGO | S_IWUSR},
> + .store = stacker_unload_write,
> +};


Apart from this, looks good. I ran it against our regression tests using
AppArmor (SubDomain) composed with Capability and everything was functionally
as expected. I still need to run it through our SMP stress tests.

Thanks

Tony
--- stacker.c.orig 2005-07-03 19:57:21.000000000 -0700
+++ stacker.c 2005-07-03 19:55:40.000000000 -0700
@@ -39,6 +39,7 @@
struct security_operations module_operations;
};
static struct list_head stacked_modules; /* list of stacked modules */
+static struct list_head unloaded_modules; /* list of unloaded modules */
static struct list_head all_modules; /* list of all modules, including freed */

static short sysfsfiles_registered;
@@ -1439,6 +1440,7 @@

rcu_read_lock();
list_del_rcu(&m->lsm_list);
+ list_add_tail_rcu(&m->lsm_list, &unloaded_modules);
if (list_empty(&stacked_modules)) {
INIT_LIST_HEAD(&default_module.lsm_list);
list_add_tail_rcu(&default_module.lsm_list, &stacked_modules);
@@ -1452,9 +1454,26 @@
return ret;
}

+/* list unloaded modules */
+static ssize_t stacker_unload_read (struct stacker_kobj *obj, char *buff)
+{
+ ssize_t len = 0;
+ struct module_entry *m;
+
+ rcu_read_lock();
+ stack_for_each_entry(m, &unloaded_modules, lsm_list) {
+ len += snprintf(buff+len, PAGE_SIZE - len, "%s\n",
+ m->module_name);
+ }
+ rcu_read_unlock();
+
+ return len;
+}
+
static struct stacker_attribute stacker_attr_unload = {
.attr = {.name = "unload", .mode = S_IFREG | S_IRUGO | S_IWUSR},
.store = stacker_unload_write,
+ .show = stacker_unload_read,
};


@@ -1525,6 +1544,7 @@

INIT_LIST_HEAD(&stacked_modules);
INIT_LIST_HEAD(&all_modules);
+ INIT_LIST_HEAD(&unloaded_modules);
spin_lock_init(&stacker_lock);
default_module.module_name = DEFAULT_MODULE_NAME;
default_module.namelen = strlen(DEFAULT_MODULE_NAME);