Changeset ef4218f in mainline

Timestamp:

2018-11-09T17:54:05Z (6 years ago)

Author:

Jakub Jermar <jakub@…>

Branches:

lfn, master, serial, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

1a5fe4f

Parents:

d19b3fc

Message:

Do not leak futexes

File:

• kernel/generic/src/synch/futex.c (modified) (9 diffs)

kernel/generic/src/synch/futex.c

@@ -45 +45 @@
  * encountered before). Futex object's lifetime is governed by
  * a reference count that represents the number of all the different
- * user space virtual addresses from all tasks that map to the
- * physical address of the futex variable. A futex object is freed
+ * tasks that reference the futex variable. A futex object is freed
  * when the last task having accessed the futex exits.
  *
@@ -75 +74 @@
 /** Task specific pointer to a global kernel futex object. */
 typedef struct futex_ptr {
-        /** List of all futex pointers used by the task. */
-        link_t all_link;
+        /** Link for the list of all futex pointers used by a task. */
+        link_t task_link;
         /** Kernel futex object. */
         futex_t *futex;
-        /** User space virtual address of the futex variable in the task. */
-        uintptr_t uaddr;
 } futex_ptr_t;
 
@@ -136 +133 @@
 
         list_foreach_safe(TASK->futex_list, cur_link, next_link) {
-                futex_ptr_t *fut_ptr = member_to_inst(cur_link, futex_ptr_t, all_link);
-
-                /*
-                 * The function is free to free the futex.  Moreover
-                 * release_ref() only frees the futex if this is the last task
-                 * referencing the futex. Therefore, only threads of this task
-                 * may have referenced the futex if it is to be freed.
-                 */
-                futex_release_ref_locked(fut_ptr->futex);
+                futex_ptr_t *futex_ptr = member_to_inst(cur_link, futex_ptr_t,
+                    task_link);
+
+                futex_release_ref_locked(futex_ptr->futex);
+                free(futex_ptr);
         }
 
@@ -166 +159 @@
 {
         assert(spinlock_locked(&futex_ht_lock));
-        assert(0 < futex->refcount);
+        assert(futex->refcount > 0);
         ++futex->refcount;
 }
@@ -174 +167 @@
 {
         assert(spinlock_locked(&futex_ht_lock));
-        assert(0 < futex->refcount);
+        assert(futex->refcount > 0);
 
         --futex->refcount;
 
-        if (0 == futex->refcount) {
+        if (futex->refcount == 0)
                 hash_table_remove(&futex_ht, &futex->paddr);
-        }
 }
 
@@ -202 +194 @@
         if (!futex)
                 return NULL;
+
+        futex_ptr_t *futex_ptr = malloc(sizeof(futex_ptr_t));
+        if (!futex_ptr) {
+                free(futex);
+                return NULL;
+        }
 
         /*
@@ -207 +205 @@
          * if it is not present).
          */
+        spinlock_lock(&TASK->futex_list_lock);
         spinlock_lock(&futex_ht_lock);
 
@@ -214 +213 @@
                 free(futex);
                 futex = member_to_inst(fut_link, futex_t, ht_link);
-                futex_add_ref(futex);
+
+                /*
+                 * See if the futex is already known to the TASK
+                 */
+                bool found = false;
+                list_foreach(TASK->futex_list, task_link, futex_ptr_t, fp) {
+                        if (fp->futex->paddr == paddr) {
+                                found = true;
+                                break;
+                        }
+                }
+                /*
+                 * If not, put it on the TASK->futex_list and bump its reference
+                 * count
+                 */
+                if (!found) {
+                        list_append(&futex_ptr->task_link, &TASK->futex_list);
+                        futex_add_ref(futex);
+                } else
+                        free(futex_ptr);
         } else {
                 futex_initialize(futex, paddr);
                 hash_table_insert(&futex_ht, &futex->ht_link);
+
+                /*
+                 * This is a new futex, so it is not on the TASK->futex_list yet
+                 */
+                futex_ptr->futex = futex;
+                list_append(&futex_ptr->task_link, &TASK->futex_list);
         }
 
         spinlock_unlock(&futex_ht_lock);
+        spinlock_unlock(&TASK->futex_list_lock);
 
         return futex;
@@ -250 +275 @@
 
 /** Sleep in futex wait queue with a timeout.
+ *
  *  If the sleep times out or is interrupted, the next wakeup is ignored.
  *  The userspace portion of the call must handle this condition.