TOMOYO: Fix lockdep warning.

Currently TOMOYO holds SRCU lock upon open() and releases it upon close()
because list elements stored in the "struct tomoyo_io_buffer" instances are
accessed until close() is called. However, such SRCU usage causes lockdep to
complain about leaving the kernel with SRCU lock held.

This patch solves the warning by holding/releasing SRCU upon each
read()/write(). This patch is doing something similar to calling kfree()
without calling synchronize_srcu(), by selectively deferring kfree() by keeping
track of the "struct tomoyo_io_buffer" instances.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: James Morris <jmorris@namei.org>

3 files changed, 276 insertions, 51 deletions

diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c
index 50481d2cf97..691c34025a4 100644
--- a/security/tomoyo/common.c
+++ b/security/tomoyo/common.c
@@ -1820,9 +1820,7 @@ static void tomoyo_read_self_domain(struct tomoyo_io_buffer *head)
  * @type: Type of interface.
  * @file: Pointer to "struct file".
  *
- * Associates policy handler and returns 0 on success, -ENOMEM otherwise.
- *
- * Caller acquires tomoyo_read_lock().
+ * Returns 0 on success, negative value otherwise.
  */
 int tomoyo_open_control(const u8 type, struct file *file)
 {
@@ -1921,9 +1919,6 @@ int tomoyo_open_control(const u8 type, struct file *file)
                         return -ENOMEM;
                 }
         }
-        if (type != TOMOYO_QUERY && type != TOMOYO_AUDIT)
-                head->reader_idx = tomoyo_read_lock();
-        file->private_data = head;
         /*
          * If the file is /sys/kernel/security/tomoyo/query , increment the
          * observer counter.
@@ -1932,6 +1927,8 @@ int tomoyo_open_control(const u8 type, struct file *file)
          */
         if (type == TOMOYO_QUERY)
                 atomic_inc(&tomoyo_query_observers);
+        file->private_data = head;
+        tomoyo_notify_gc(head, true);
         return 0;
 }
 
@@ -2000,13 +1997,12 @@ static inline bool tomoyo_has_more_namespace(struct tomoyo_io_buffer *head)
  * @buffer_len: Size of @buffer.
  *
  * Returns bytes read on success, negative value otherwise.
- *
- * Caller holds tomoyo_read_lock().
  */
 int tomoyo_read_control(struct tomoyo_io_buffer *head, char __user *buffer,
                         const int buffer_len)
 {
         int len;
+        int idx;
 
         if (!head->read)
                 return -ENOSYS;
@@ -2014,6 +2010,7 @@ int tomoyo_read_control(struct tomoyo_io_buffer *head, char __user *buffer,
                 return -EINTR;
         head->read_user_buf = buffer;
         head->read_user_buf_avail = buffer_len;
+        idx = tomoyo_read_lock();
         if (tomoyo_flush(head))
                 /* Call the policy handler. */
                 do {
@@ -2021,6 +2018,7 @@ int tomoyo_read_control(struct tomoyo_io_buffer *head, char __user *buffer,
                         head->read(head);
                 } while (tomoyo_flush(head) &&
                          tomoyo_has_more_namespace(head));
+        tomoyo_read_unlock(idx);
         len = head->read_user_buf - buffer;
         mutex_unlock(&head->io_sem);
         return len;
@@ -2071,8 +2069,6 @@ static int tomoyo_parse_policy(struct tomoyo_io_buffer *head, char *line)
  * @buffer_len: Size of @buffer.
  *
  * Returns @buffer_len on success, negative value otherwise.
- *
- * Caller holds tomoyo_read_lock().
  */
 int tomoyo_write_control(struct tomoyo_io_buffer *head,
                          const char __user *buffer, const int buffer_len)
@@ -2080,12 +2076,14 @@ int tomoyo_write_control(struct tomoyo_io_buffer *head,
         int error = buffer_len;
         size_t avail_len = buffer_len;
         char *cp0 = head->write_buf;
+        int idx;
         if (!head->write)
                 return -ENOSYS;
         if (!access_ok(VERIFY_READ, buffer, buffer_len))
                 return -EFAULT;
         if (mutex_lock_interruptible(&head->io_sem))
                 return -EINTR;
+        idx = tomoyo_read_lock();
         /* Read a line and dispatch it to the policy handler. */
         while (avail_len > 0) {
                 char c;
@@ -2148,6 +2146,7 @@ int tomoyo_write_control(struct tomoyo_io_buffer *head,
                 }
         }
 out:
+        tomoyo_read_unlock(idx);
         mutex_unlock(&head->io_sem);
         return error;
 }
@@ -2157,30 +2156,18 @@ out:
  *
  * @head: Pointer to "struct tomoyo_io_buffer".
  *
- * Releases memory and returns 0.
- *
- * Caller looses tomoyo_read_lock().
+ * Returns 0.
  */
 int tomoyo_close_control(struct tomoyo_io_buffer *head)
 {
-        const bool is_write = !!head->write_buf;
-
         /*
          * If the file is /sys/kernel/security/tomoyo/query , decrement the
          * observer counter.
          */
-        if (head->type == TOMOYO_QUERY)
-                atomic_dec(&tomoyo_query_observers);
-        else if (head->type != TOMOYO_AUDIT)
-                tomoyo_read_unlock(head->reader_idx);
-        /* Release memory used for policy I/O. */
-        kfree(head->read_buf);
-        head->read_buf = NULL;
-        kfree(head->write_buf);
-        head->write_buf = NULL;
-        kfree(head);
-        if (is_write)
-                tomoyo_run_gc();
+        if (head->type == TOMOYO_QUERY &&
+            atomic_dec_and_test(&tomoyo_query_observers))
+                wake_up_all(&tomoyo_answer_wait);
+        tomoyo_notify_gc(head, false);
         return 0;
 }
 
diff --git a/security/tomoyo/common.h b/security/tomoyo/common.h
index 53c8798e38b..a5eeabcc073 100644
--- a/security/tomoyo/common.h
+++ b/security/tomoyo/common.h
@@ -441,8 +441,6 @@ struct tomoyo_io_buffer {
         int (*poll) (struct file *file, poll_table *wait);
         /* Exclusive lock for this structure.   */
         struct mutex io_sem;
-        /* Index returned by tomoyo_read_lock(). */
-        int reader_idx;
         char __user *read_user_buf;
         int read_user_buf_avail;
         struct {
@@ -480,6 +478,10 @@ struct tomoyo_io_buffer {
         int writebuf_size;
         /* Type of this interface.              */
         u8 type;
+        /* Users counter protected by tomoyo_io_buffer_list_lock. */
+        u8 users;
+        /* List for telling GC not to kfree() elements. */
+        struct list_head list;
 };
 
 /*
@@ -651,7 +653,7 @@ int tomoyo_find_next_domain(struct linux_binprm *bprm);
 void tomoyo_print_ulong(char *buffer, const int buffer_len,
                         const unsigned long value, const u8 type);
 void tomoyo_put_name_union(struct tomoyo_name_union *ptr);
-void tomoyo_run_gc(void);
+void tomoyo_notify_gc(struct tomoyo_io_buffer *head, const bool is_register);
 void tomoyo_memory_free(void *ptr);
 int tomoyo_update_domain(struct tomoyo_acl_info *new_entry, const int size,
                          struct tomoyo_acl_param *param,
diff --git a/security/tomoyo/gc.c b/security/tomoyo/gc.c
index 782e844dca7..1e1a6c8c832 100644
--- a/security/tomoyo/gc.c
+++ b/security/tomoyo/gc.c
@@ -11,13 +11,123 @@
 #include <linux/kthread.h>
 #include <linux/slab.h>
 
+/* The list for "struct tomoyo_io_buffer". */
+static LIST_HEAD(tomoyo_io_buffer_list);
+/* Lock for protecting tomoyo_io_buffer_list. */
+static DEFINE_SPINLOCK(tomoyo_io_buffer_list_lock);
+
+/* Size of an element. */
+static const u8 tomoyo_element_size[TOMOYO_MAX_POLICY] = {
+        [TOMOYO_ID_GROUP] = sizeof(struct tomoyo_group),
+        [TOMOYO_ID_PATH_GROUP] = sizeof(struct tomoyo_path_group),
+        [TOMOYO_ID_NUMBER_GROUP] = sizeof(struct tomoyo_number_group),
+        [TOMOYO_ID_AGGREGATOR] = sizeof(struct tomoyo_aggregator),
+        [TOMOYO_ID_TRANSITION_CONTROL] =
+        sizeof(struct tomoyo_transition_control),
+        [TOMOYO_ID_MANAGER] = sizeof(struct tomoyo_manager),
+        /* [TOMOYO_ID_NAME] = "struct tomoyo_name"->size, */
+        /* [TOMOYO_ID_ACL] =
+           tomoyo_acl_size["struct tomoyo_acl_info"->type], */
+        [TOMOYO_ID_DOMAIN] = sizeof(struct tomoyo_domain_info),
+};
+
+/* Size of a domain ACL element. */
+static const u8 tomoyo_acl_size[] = {
+        [TOMOYO_TYPE_PATH_ACL] = sizeof(struct tomoyo_path_acl),
+        [TOMOYO_TYPE_PATH2_ACL] = sizeof(struct tomoyo_path2_acl),
+        [TOMOYO_TYPE_PATH_NUMBER_ACL] = sizeof(struct tomoyo_path_number_acl),
+        [TOMOYO_TYPE_MKDEV_ACL] = sizeof(struct tomoyo_mkdev_acl),
+        [TOMOYO_TYPE_MOUNT_ACL] = sizeof(struct tomoyo_mount_acl),
+};
+
+/**
+ * tomoyo_struct_used_by_io_buffer - Check whether the list element is used by /sys/kernel/security/tomoyo/ users or not.
+ *
+ * @element: Pointer to "struct list_head".
+ *
+ * Returns true if @element is used by /sys/kernel/security/tomoyo/ users,
+ * false otherwise.
+ */
+static bool tomoyo_struct_used_by_io_buffer(const struct list_head *element)
+{
+        struct tomoyo_io_buffer *head;
+        bool in_use = false;
+
+        spin_lock(&tomoyo_io_buffer_list_lock);
+        list_for_each_entry(head, &tomoyo_io_buffer_list, list) {
+                head->users++;
+                spin_unlock(&tomoyo_io_buffer_list_lock);
+                if (mutex_lock_interruptible(&head->io_sem)) {
+                        in_use = true;
+                        goto out;
+                }
+                if (head->r.domain == element || head->r.group == element ||
+                    head->r.acl == element || &head->w.domain->list == element)
+                        in_use = true;
+                mutex_unlock(&head->io_sem);
+out:
+                spin_lock(&tomoyo_io_buffer_list_lock);
+                head->users--;
+                if (in_use)
+                        break;
+        }
+        spin_unlock(&tomoyo_io_buffer_list_lock);
+        return in_use;
+}
+
+/**
+ * tomoyo_name_used_by_io_buffer - Check whether the string is used by /sys/kernel/security/tomoyo/ users or not.
+ *
+ * @string: String to check.
+ * @size:   Memory allocated for @string .
+ *
+ * Returns true if @string is used by /sys/kernel/security/tomoyo/ users,
+ * false otherwise.
+ */
+static bool tomoyo_name_used_by_io_buffer(const char *string,
+                                          const size_t size)
+{
+        struct tomoyo_io_buffer *head;
+        bool in_use = false;
+
+        spin_lock(&tomoyo_io_buffer_list_lock);
+        list_for_each_entry(head, &tomoyo_io_buffer_list, list) {
+                int i;
+                head->users++;
+                spin_unlock(&tomoyo_io_buffer_list_lock);
+                if (mutex_lock_interruptible(&head->io_sem)) {
+                        in_use = true;
+                        goto out;
+                }
+                for (i = 0; i < TOMOYO_MAX_IO_READ_QUEUE; i++) {
+                        const char *w = head->r.w[i];
+                        if (w < string || w > string + size)
+                                continue;
+                        in_use = true;
+                        break;
+                }
+                mutex_unlock(&head->io_sem);
+out:
+                spin_lock(&tomoyo_io_buffer_list_lock);
+                head->users--;
+                if (in_use)
+                        break;
+        }
+        spin_unlock(&tomoyo_io_buffer_list_lock);
+        return in_use;
+}
+
+/* Structure for garbage collection. */
 struct tomoyo_gc {
         struct list_head list;
         enum tomoyo_policy_id type;
+        size_t size;
         struct list_head *element;
 };
-static LIST_HEAD(tomoyo_gc_queue);
-static DEFINE_MUTEX(tomoyo_gc_mutex);
+/* List of entries to be deleted. */
+static LIST_HEAD(tomoyo_gc_list);
+/* Length of tomoyo_gc_list. */
+static int tomoyo_gc_list_len;
 
 /**
  * tomoyo_add_to_gc - Add an entry to to be deleted list.
@@ -43,10 +153,42 @@ static bool tomoyo_add_to_gc(const int type, struct list_head *element)
         if (!entry)
                 return false;
         entry->type = type;
+        if (type == TOMOYO_ID_ACL)
+                entry->size = tomoyo_acl_size[
+                              container_of(element,
+                                           typeof(struct tomoyo_acl_info),
+                                           list)->type];
+        else if (type == TOMOYO_ID_NAME)
+                entry->size = strlen(container_of(element,
+                                                  typeof(struct tomoyo_name),
+                                                  head.list)->entry.name) + 1;
+        else
+                entry->size = tomoyo_element_size[type];
         entry->element = element;
-        list_add(&entry->list, &tomoyo_gc_queue);
+        list_add(&entry->list, &tomoyo_gc_list);
         list_del_rcu(element);
-        return true;
+        return tomoyo_gc_list_len++ < 128;
+}
+
+/**
+ * tomoyo_element_linked_by_gc - Validate next element of an entry.
+ *
+ * @element: Pointer to an element.
+ * @size:    Size of @element in byte.
+ *
+ * Returns true if @element is linked by other elements in the garbage
+ * collector's queue, false otherwise.
+ */
+static bool tomoyo_element_linked_by_gc(const u8 *element, const size_t size)
+{
+        struct tomoyo_gc *p;
+        list_for_each_entry(p, &tomoyo_gc_list, list) {
+                const u8 *ptr = (const u8 *) p->element->next;
+                if (ptr < element || element + size < ptr)
+                        continue;
+                return true;
+        }
+        return false;
 }
 
 /**
@@ -151,6 +293,13 @@ static void tomoyo_del_acl(struct list_head *element)
         }
 }
 
+/**
+ * tomoyo_del_domain - Delete members in "struct tomoyo_domain_info".
+ *
+ * @element: Pointer to "struct list_head".
+ *
+ * Returns true if deleted, false otherwise.
+ */
 static bool tomoyo_del_domain(struct list_head *element)
 {
         struct tomoyo_domain_info *domain =
@@ -360,13 +509,44 @@ unlock:
         mutex_unlock(&tomoyo_policy_lock);
 }
 
-static void tomoyo_kfree_entry(void)
+/**
+ * tomoyo_kfree_entry - Delete entries in tomoyo_gc_list.
+ *
+ * Returns true if some entries were kfree()d, false otherwise.
+ */
+static bool tomoyo_kfree_entry(void)
 {
         struct tomoyo_gc *p;
         struct tomoyo_gc *tmp;
+        bool result = false;
 
-        list_for_each_entry_safe(p, tmp, &tomoyo_gc_queue, list) {
+        list_for_each_entry_safe(p, tmp, &tomoyo_gc_list, list) {
                 struct list_head *element = p->element;
+
+                /*
+                 * list_del_rcu() in tomoyo_add_to_gc() guarantees that the
+                 * list element became no longer reachable from the list which
+                 * the element was originally on (e.g. tomoyo_domain_list).
+                 * Also, synchronize_srcu() in tomoyo_gc_thread() guarantees
+                 * that the list element became no longer referenced by syscall
+                 * users.
+                 *
+                 * However, there are three users which may still be using the
+                 * list element. We need to defer until all of these users
+                 * forget the list element.
+                 *
+                 * Firstly, defer until "struct tomoyo_io_buffer"->r.{domain,
+                 * group,acl} and "struct tomoyo_io_buffer"->w.domain forget
+                 * the list element.
+                 */
+                if (tomoyo_struct_used_by_io_buffer(element))
+                        continue;
+                /*
+                 * Secondly, defer until all other elements in the
+                 * tomoyo_gc_list list forget the list element.
+                 */
+                if (tomoyo_element_linked_by_gc((const u8 *) element, p->size))
+                        continue;
                 switch (p->type) {
                 case TOMOYO_ID_TRANSITION_CONTROL:
                         tomoyo_del_transition_control(element);
@@ -378,6 +558,14 @@ static void tomoyo_kfree_entry(void)
                         tomoyo_del_manager(element);
                         break;
                 case TOMOYO_ID_NAME:
+                        /*
+                         * Thirdly, defer until all "struct tomoyo_io_buffer"
+                         * ->r.w[] forget the list element.
+                         */
+                        if (tomoyo_name_used_by_io_buffer(
+                            container_of(element, typeof(struct tomoyo_name),
+                                         head.list)->entry.name, p->size))
+                                continue;
                         tomoyo_del_name(element);
                         break;
                 case TOMOYO_ID_ACL:
@@ -402,7 +590,10 @@ static void tomoyo_kfree_entry(void)
                 tomoyo_memory_free(element);
                 list_del(&p->list);
                 kfree(p);
+                tomoyo_gc_list_len--;
+                result = true;
         }
+        return result;
 }
 
 /**
@@ -418,25 +609,70 @@ static void tomoyo_kfree_entry(void)
  */
 static int tomoyo_gc_thread(void *unused)
 {
+        /* Garbage collector thread is exclusive. */
+        static DEFINE_MUTEX(tomoyo_gc_mutex);
+        if (!mutex_trylock(&tomoyo_gc_mutex))
+                goto out;
         daemonize("GC for TOMOYO");
-        if (mutex_trylock(&tomoyo_gc_mutex)) {
-                int i;
-                for (i = 0; i < 10; i++) {
-                        tomoyo_collect_entry();
-                        if (list_empty(&tomoyo_gc_queue))
-                                break;
-                        synchronize_srcu(&tomoyo_ss);
-                        tomoyo_kfree_entry();
+        do {
+                tomoyo_collect_entry();
+                if (list_empty(&tomoyo_gc_list))
+                        break;
+                synchronize_srcu(&tomoyo_ss);
+        } while (tomoyo_kfree_entry());
+        {
+                struct tomoyo_io_buffer *head;
+                struct tomoyo_io_buffer *tmp;
+
+                spin_lock(&tomoyo_io_buffer_list_lock);
+                list_for_each_entry_safe(head, tmp, &tomoyo_io_buffer_list,
+                                         list) {
+                        if (head->users)
+                                continue;
+                        list_del(&head->list);
+                        kfree(head->read_buf);
+                        kfree(head->write_buf);
+                        kfree(head);
                 }
-                mutex_unlock(&tomoyo_gc_mutex);
+                spin_unlock(&tomoyo_io_buffer_list_lock);
         }
-        do_exit(0);
+        mutex_unlock(&tomoyo_gc_mutex);
+out:
+        /* This acts as do_exit(0). */
+        return 0;
 }
 
-void tomoyo_run_gc(void)
+/**
+ * tomoyo_notify_gc - Register/unregister /sys/kernel/security/tomoyo/ users.
+ *
+ * @head:        Pointer to "struct tomoyo_io_buffer".
+ * @is_register: True if register, false if unregister.
+ *
+ * Returns nothing.
+ */
+void tomoyo_notify_gc(struct tomoyo_io_buffer *head, const bool is_register)
 {
-        struct task_struct *task = kthread_create(tomoyo_gc_thread, NULL,
-                                                  "GC for TOMOYO");
-        if (!IS_ERR(task))
-                wake_up_process(task);
+        bool is_write = false;
+
+        spin_lock(&tomoyo_io_buffer_list_lock);
+        if (is_register) {
+                head->users = 1;
+                list_add(&head->list, &tomoyo_io_buffer_list);
+        } else {
+                is_write = head->write_buf != NULL;
+                if (!--head->users) {
+                        list_del(&head->list);
+                        kfree(head->read_buf);
+                        kfree(head->write_buf);
+                        kfree(head);
+                }
+        }
+        spin_unlock(&tomoyo_io_buffer_list_lock);
+        if (is_write) {
+                struct task_struct *task = kthread_create(tomoyo_gc_thread,
+                                                          NULL,
+                                                          "GC for TOMOYO");
+                if (!IS_ERR(task))
+                        wake_up_process(task);
+        }
 }