IMA: policy can now be updated multiple times

The new rules get appended to the original policy, forming a queue.
The new rules are first added to a temporary list, which on error
get released without disturbing the normal IMA operations.  On
success both lists (the current policy and the new rules) are spliced.

IMA policy reads are many orders of magnitude more numerous compared to
writes, the match code is RCU protected.  The updater side also does
list splice in RCU manner.

Signed-off-by: Petko Manolov <petkan@mip-labs.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>

3 files changed, 75 insertions, 28 deletions

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index a292b881c16f..e74d66cbfe87 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -107,6 +107,17 @@ config IMA_DEFAULT_HASH
         default "sha512" if IMA_DEFAULT_HASH_SHA512
         default "wp512" if IMA_DEFAULT_HASH_WP512
 
+config IMA_WRITE_POLICY
+        bool "Enable multiple writes to the IMA policy"
+        depends on IMA
+        default n
+        help
+          IMA policy can now be updated multiple times.  The new rules get
+          appended to the original policy.  Have in mind that the rules are
+          scanned in FIFO order so be careful when you design and add new ones.
+
+          If unsure, say N.
+
 config IMA_APPRAISE
         bool "Appraise integrity measurements"
         depends on IMA
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 816d175da79a..a3cf5c0ab501 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -25,6 +25,8 @@
 
 #include "ima.h"
 
+static DEFINE_MUTEX(ima_write_mutex);
+
 static int valid_policy = 1;
 #define TMPBUFLEN 12
 static ssize_t ima_show_htable_value(char __user *buf, size_t count,
@@ -261,6 +263,11 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
 {
         char *data = NULL;
         ssize_t result;
+        int res;
+
+        res = mutex_lock_interruptible(&ima_write_mutex);
+        if (res)
+                return res;
 
         if (datalen >= PAGE_SIZE)
                 datalen = PAGE_SIZE - 1;
@@ -286,6 +293,8 @@ out:
         if (result < 0)
                 valid_policy = 0;
         kfree(data);
+        mutex_unlock(&ima_write_mutex);
+
         return result;
 }
 
@@ -337,8 +346,12 @@ static int ima_release_policy(struct inode *inode, struct file *file)
                 return 0;
         }
         ima_update_policy();
+#ifndef CONFIG_IMA_WRITE_POLICY
         securityfs_remove(ima_policy);
         ima_policy = NULL;
+#else
+        clear_bit(IMA_FS_BUSY, &ima_fs_flags);
+#endif
         return 0;
 }
 
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 3997e206f82d..10a0a9b9e22d 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -16,6 +16,7 @@
 #include <linux/magic.h>
 #include <linux/parser.h>
 #include <linux/slab.h>
+#include <linux/rculist.h>
 #include <linux/genhd.h>
 
 #include "ima.h"
@@ -135,11 +136,11 @@ static struct ima_rule_entry default_appraise_rules[] = {
 
 static LIST_HEAD(ima_default_rules);
 static LIST_HEAD(ima_policy_rules);
+static LIST_HEAD(ima_temp_rules);
 static struct list_head *ima_rules;
 
-static DEFINE_MUTEX(ima_rules_mutex);
-
 static int ima_policy __initdata;
+
 static int __init default_measure_policy_setup(char *str)
 {
         if (ima_policy)
@@ -171,21 +172,18 @@ static int __init default_appraise_policy_setup(char *str)
 __setup("ima_appraise_tcb", default_appraise_policy_setup);
 
 /*
- * Although the IMA policy does not change, the LSM policy can be
- * reloaded, leaving the IMA LSM based rules referring to the old,
- * stale LSM policy.
- *
- * Update the IMA LSM based rules to reflect the reloaded LSM policy.
- * We assume the rules still exist; and BUG_ON() if they don't.
+ * The LSM policy can be reloaded, leaving the IMA LSM based rules referring
+ * to the old, stale LSM policy.  Update the IMA LSM based rules to reflect
+ * the reloaded LSM policy.  We assume the rules still exist; and BUG_ON() if
+ * they don't.
  */
 static void ima_lsm_update_rules(void)
 {
-        struct ima_rule_entry *entry, *tmp;
+        struct ima_rule_entry *entry;
         int result;
         int i;
 
-        mutex_lock(&ima_rules_mutex);
-        list_for_each_entry_safe(entry, tmp, &ima_policy_rules, list) {
+        list_for_each_entry(entry, &ima_policy_rules, list) {
                 for (i = 0; i < MAX_LSM_RULES; i++) {
                         if (!entry->lsm[i].rule)
                                 continue;
@@ -196,7 +194,6 @@ static void ima_lsm_update_rules(void)
                         BUG_ON(!entry->lsm[i].rule);
                 }
         }
-        mutex_unlock(&ima_rules_mutex);
 }
 
 /**
@@ -319,9 +316,9 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
  * Measure decision based on func/mask/fsmagic and LSM(subj/obj/type)
  * conditions.
  *
- * (There is no need for locking when walking the policy list,
- * as elements in the list are never deleted, nor does the list
- * change.)
+ * Since the IMA policy may be updated multiple times we need to lock the
+ * list when walking it.  Reads are many orders of magnitude more numerous
+ * than writes so ima_match_policy() is classical RCU candidate.
  */
 int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
                      int flags)
@@ -329,7 +326,8 @@ int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
         struct ima_rule_entry *entry;
         int action = 0, actmask = flags | (flags << 1);
 
-        list_for_each_entry(entry, ima_rules, list) {
+        rcu_read_lock();
+        list_for_each_entry_rcu(entry, ima_rules, list) {
 
                 if (!(entry->action & actmask))
                         continue;
@@ -351,6 +349,7 @@ int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
                 if (!actmask)
                         break;
         }
+        rcu_read_unlock();
 
         return action;
 }
@@ -365,7 +364,6 @@ void ima_update_policy_flag(void)
 {
         struct ima_rule_entry *entry;
 
-        ima_policy_flag = 0;
         list_for_each_entry(entry, ima_rules, list) {
                 if (entry->action & IMA_DO_MASK)
                         ima_policy_flag |= entry->action;
@@ -419,12 +417,36 @@ void __init ima_init_policy(void)
  * ima_update_policy - update default_rules with new measure rules
  *
  * Called on file .release to update the default rules with a complete new
- * policy.  Once updated, the policy is locked, no additional rules can be
- * added to the policy.
+ * policy.  What we do here is to splice ima_policy_rules and ima_temp_rules so
+ * they make a queue.  The policy may be updated multiple times and this is the
+ * RCU updater.
+ *
+ * Policy rules are never deleted so ima_policy_flag gets zeroed only once when
+ * we switch from the default policy to user defined.
  */
 void ima_update_policy(void)
 {
-        ima_rules = &ima_policy_rules;
+        struct list_head *first, *last, *policy;
+
+        /* append current policy with the new rules */
+        first = (&ima_temp_rules)->next;
+        last = (&ima_temp_rules)->prev;
+        policy = &ima_policy_rules;
+
+        synchronize_rcu();
+
+        last->next = policy;
+        rcu_assign_pointer(list_next_rcu(policy->prev), first);
+        first->prev = policy->prev;
+        policy->prev = last;
+
+        /* prepare for the next policy rules addition */
+        INIT_LIST_HEAD(&ima_temp_rules);
+
+        if (ima_rules != policy) {
+                ima_policy_flag = 0;
+                ima_rules = policy;
+        }
         ima_update_policy_flag();
 }
 
@@ -746,7 +768,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
  * ima_parse_add_rule - add a rule to ima_policy_rules
  * @rule - ima measurement policy rule
  *
- * Uses a mutex to protect the policy list from multiple concurrent writers.
+ * Avoid locking by allowing just one writer at a time in ima_write_policy()
  * Returns the length of the rule parsed, an error code on failure
  */
 ssize_t ima_parse_add_rule(char *rule)
@@ -782,26 +804,27 @@ ssize_t ima_parse_add_rule(char *rule)
                 return result;
         }
 
-        mutex_lock(&ima_rules_mutex);
-        list_add_tail(&entry->list, &ima_policy_rules);
-        mutex_unlock(&ima_rules_mutex);
+        list_add_tail(&entry->list, &ima_temp_rules);
 
         return len;
 }
 
-/* ima_delete_rules called to cleanup invalid policy */
+/**
+ * ima_delete_rules() called to cleanup invalid in-flight policy.
+ * We don't need locking as we operate on the temp list, which is
+ * different from the active one.  There is also only one user of
+ * ima_delete_rules() at a time.
+ */
 void ima_delete_rules(void)
 {
         struct ima_rule_entry *entry, *tmp;
         int i;
 
-        mutex_lock(&ima_rules_mutex);
-        list_for_each_entry_safe(entry, tmp, &ima_policy_rules, list) {
+        list_for_each_entry_safe(entry, tmp, &ima_temp_rules, list) {
                 for (i = 0; i < MAX_LSM_RULES; i++)
                         kfree(entry->lsm[i].args_p);
 
                 list_del(&entry->list);
                 kfree(entry);
         }
-        mutex_unlock(&ima_rules_mutex);
 }