[PATCH] !!!! HERE BE DRAGONS - UNFIXED REFCOUNT ISSUES FROM PREVIOUS PATCH AND ALL UNTESTED !!!!

[Christian Brauner]

ima: get rid of user_ns member in struct ima_namespace
---
 include/linux/ima.h                      | 40 +++++++++---------------
 security/integrity/ima/ima_fs.c          |  2 +-
 security/integrity/ima/ima_init_ima_ns.c |  3 +-
 security/integrity/ima/ima_main.c        |  2 +-
 security/integrity/ima/ima_ns.c          |  9 ++----
 security/integrity/ima/ima_queue_keys.c  | 22 +++++++++----
 6 files changed, 36 insertions(+), 42 deletions(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 32bf98092143..73cdfbf3f9d4 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -13,6 +13,7 @@
 #include <linux/kexec.h>
 #include <linux/user_namespace.h>
 #include <crypto/hash_info.h>
+#include <linux/refcount.h>
 struct linux_binprm;
 
 #ifdef CONFIG_IMA
@@ -241,8 +242,6 @@ enum {
 };
 
 struct ima_namespace {
-	struct kref kref;
-	struct user_namespace *user_ns;
 	struct rb_root ns_status_tree;
 	rwlock_t ns_status_lock;
 	struct kmem_cache *ns_status_cache;
@@ -264,6 +263,7 @@ struct ima_namespace {
 	 * for measurement should be freed. This worker is used
 	 * for handling this scenario.
 	 */
+	refcount_t ima_keys_delayed_ref;
 	struct delayed_work ima_keys_delayed_work;
 	long ima_key_queue_timeout;
 	bool timer_expired;
@@ -295,24 +295,12 @@ extern struct list_head ima_default_rules;
 
 #ifdef CONFIG_IMA_NS
 
-void free_ima_ns(struct kref *kref);
-
-static inline struct ima_namespace *get_ima_ns(struct ima_namespace *ns)
-{
-	if (ns)
-		kref_get(&ns->kref);
-
-	return ns;
-}
+void free_ima_ns(struct ima_namespace *ns);
+void __put_delayed_ima_ns(struct ima_namespace *ns);
 
 static inline void put_ima_ns(struct user_namespace *user_ns)
 {
-	struct ima_namespace *ns = user_ns->ima_ns;
-
-	if (ns) {
-		pr_debug("DEREF   ima_ns: 0x%p  ctr: %d\n", ns, kref_read(&ns->kref));
-		kref_put(&ns->kref, free_ima_ns);
-	}
+	__put_delayed_ima_ns(user_ns->ima_ns);
 }
 
 int create_ima_ns(struct user_namespace *user_ns);
@@ -322,21 +310,20 @@ static inline struct ima_namespace *get_current_ns(void)
 	return current_user_ns()->ima_ns;
 }
 
-#else
-
-static inline struct ima_namespace *get_ima_ns(struct ima_namespace *ns)
+static inline struct user_namespace *ima_user_ns(const struct ima_namespace *ima_ns)
 {
-	return ns;
+	struct user_namespace *user_ns;
+	user_ns = current_user_ns();
+	WARN_ON(user_ns->ima_ns != ima_ns);
+	return user_ns;
 }
 
-static inline void put_ima_ns(struct user_namespace *user_ns)
-{
-}
+#else
 
 static inline int create_ima_ns(struct user_namespace *user_ns)
 {
 #if CONFIG_IMA
-	user_ns->ima_ns = get_ima_ns(&init_ima_ns);
+	user_ns->ima_ns = &init_ima_ns;
 #endif
 	return 0;
 }
@@ -346,6 +333,9 @@ static inline struct ima_namespace *get_current_ns(void)
 	return &init_ima_ns;
 }
 
+static inline void put_ima_ns(struct user_namespace *user_ns)
+{
+}
 #endif /* CONFIG_IMA_NS */
 
 #if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING)
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 778983fd9a73..583462b29cb5 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -386,7 +386,7 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
 #else
 		if ((filp->f_flags & O_ACCMODE) != O_RDONLY)
 			return -EACCES;
-		if (!mac_admin_ns_capable(ns->user_ns))
+		if (!mac_admin_ns_capable(ima_user_ns(ns)))
 			return -EPERM;
 		return seq_open(filp, &ima_policy_seqops);
 #endif
diff --git a/security/integrity/ima/ima_init_ima_ns.c b/security/integrity/ima/ima_init_ima_ns.c
index 162c94e06d13..6ae6df037f03 100644
--- a/security/integrity/ima/ima_init_ima_ns.c
+++ b/security/integrity/ima/ima_init_ima_ns.c
@@ -62,12 +62,11 @@ int __init ima_ns_init(void)
 }
 
 struct ima_namespace init_ima_ns = {
-	.kref = KREF_INIT(1),
-	.user_ns = &init_user_ns,
 #ifdef CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS
 	.ima_process_keys = false,
 	.ima_keys_lock = __MUTEX_INITIALIZER(init_ima_ns.ima_keys_lock),
 	.ima_keys = LIST_HEAD_INIT(init_ima_ns.ima_keys),
 #endif
+	.ima_keys_delayed_ref = REFCOUNT_INIT(1),
 };
 EXPORT_SYMBOL(init_ima_ns);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 70fa26b7bd3f..6ebc57cd91d3 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -410,7 +410,7 @@ static int process_measurement(struct ima_namespace *ns,
 			       u32 secid, char *buf, loff_t size, int mask,
 			       enum ima_hooks func)
 {
-	struct user_namespace *user_ns = ns->user_ns;
+	struct user_namespace *user_ns = ima_user_ns(ns);
 	int ret = 0;
 
 	while (user_ns) {
diff --git a/security/integrity/ima/ima_ns.c b/security/integrity/ima/ima_ns.c
index 6a0632806cdb..f96286ad0da8 100644
--- a/security/integrity/ima/ima_ns.c
+++ b/security/integrity/ima/ima_ns.c
@@ -31,9 +31,6 @@ int create_ima_ns(struct user_namespace *user_ns)
 		return -ENOMEM;
 	pr_debug("NEW     ima_ns: 0x%p\n", ns);
 
-	kref_init(&ns->kref);
-	ns->user_ns = user_ns;
-
 	err = ima_init_namespace(ns);
 	if (err)
 		goto fail_free;
@@ -44,6 +41,7 @@ int create_ima_ns(struct user_namespace *user_ns)
 	INIT_LIST_HEAD(&ns->ima_keys);
 #endif
 
+	refcount_set(&ns->ima_keys_delayed_ref, 1);
 	user_ns->ima_ns = ns;
 
 	return 0;
@@ -63,11 +61,8 @@ static void destroy_ima_ns(struct ima_namespace *ns)
 	kmem_cache_free(imans_cachep, ns);
 }
 
-void free_ima_ns(struct kref *kref)
+void free_ima_ns(struct ima_namespace *ns)
 {
-	struct ima_namespace *ns;
-
-	ns = container_of(kref, struct ima_namespace, kref);
 	if (WARN_ON(ns == &init_ima_ns))
 		return;
 
diff --git a/security/integrity/ima/ima_queue_keys.c b/security/integrity/ima/ima_queue_keys.c
index a6eb802e5ae4..d7c43e592e2c 100644
--- a/security/integrity/ima/ima_queue_keys.c
+++ b/security/integrity/ima/ima_queue_keys.c
@@ -14,6 +14,19 @@
 #include <keys/asymmetric-type.h>
 #include "ima.h"
 
+static inline void __get_delayed_ima_ns(struct ima_namespace *ima_ns)
+{
+	refcount_inc(&ima_ns->ima_keys_delayed_ref);
+}
+
+void __put_delayed_ima_ns(struct ima_namespace *ima_ns)
+{
+	if (ima_ns && refcount_dec_and_test(&ima_ns->ima_keys_delayed_ref)) {
+		pr_debug("DEREF   ima_ns: 0x%p  ctr: %d\n", ima_ns,
+			 refcount_read(&ima_ns->ima_keys_delayed_ref));
+		free_ima_ns(ima_ns);
+	}
+}
 
 /*
  * This worker function frees keys that may still be
@@ -26,8 +39,7 @@ void ima_keys_handler(struct work_struct *work)
 	ns = container_of(work, struct ima_namespace, ima_keys_delayed_work.work);
 	ns->timer_expired = true;
 	ima_process_queued_keys(ns);
-
-	put_user_ns(ns->user_ns);
+	__put_delayed_ima_ns(ns);
 }
 
 /*
@@ -36,9 +48,7 @@ void ima_keys_handler(struct work_struct *work)
  */
 void ima_init_key_queue(struct ima_namespace *ns)
 {
-	/* keep IMA namespace until delayed work is done */
-	get_user_ns(ns->user_ns);
-
+	__get_delayed_ima_ns(ns);
 	schedule_delayed_work(&ns->ima_keys_delayed_work,
 			      msecs_to_jiffies(ns->ima_key_queue_timeout));
 }
@@ -145,7 +155,7 @@ void ima_process_queued_keys(struct ima_namespace *ns)
 	if (!ns->timer_expired) {
 		if (cancel_delayed_work_sync(&ns->ima_keys_delayed_work))
 			/* undo reference from ima_init_key_queue */
-			put_user_ns(ns->user_ns);
+			__put_delayed_ima_ns(ns);
 	}
 
 	list_for_each_entry_safe(entry, tmp, &ns->ima_keys, list) {
-- 
2.30.2