userspace: remove threads from domain on abort

When threads exited we were leaving dangling references to
them in the domain's mem_domain_q.

z_thread_single_abort() now calls into the memory domain
code via z_mem_domain_exit_thread() to take it off.

The thread setup code now invokes z_mem_domain_init_thread(),
avoiding extra checks in k_mem_domain_add_thread(), we know
the object isn't currently a member of a doamin.

Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>

kernel/include/kernel_internal.h

@@ -126,6 +126,12 @@ extern uint8_t *z_priv_stack_find(k_thread_stack_t *stack);
 
 #ifdef CONFIG_USERSPACE
 bool z_stack_is_user_capable(k_thread_stack_t *stack);
+
+/* Memory domain setup hook, called from z_setup_new_thread() */
+void z_mem_domain_init_thread(struct k_thread *thread);
+
+/* Memory domain teardown hook, called from z_thread_single_abort() */
+void z_mem_domain_exit_thread(struct k_thread *thread);
 #endif /* CONFIG_USERSPACE */
 
 #ifdef CONFIG_GDBSTUB

kernel/mem_domain.c

@@ -234,23 +234,12 @@ void k_mem_domain_remove_partition(struct k_mem_domain *domain,
 	k_spin_unlock(&lock, key);
 }
 
-void k_mem_domain_add_thread(struct k_mem_domain *domain, k_tid_t thread)
+static void add_thread_locked(struct k_mem_domain *domain,
+			      k_tid_t thread)
 {
-	k_spinlock_key_t key;
-
 	__ASSERT_NO_MSG(domain != NULL);
 	__ASSERT_NO_MSG(thread != NULL);
 
-	key = k_spin_lock(&lock);
-	if (thread->mem_domain_info.mem_domain != NULL) {
-		LOG_DBG("remove thread %p from memory domain %p\n",
-			thread, thread->mem_domain_info.mem_domain);
-		sys_dlist_remove(&thread->mem_domain_info.mem_domain_q_node);
-#ifdef CONFIG_ARCH_MEM_DOMAIN_SYNCHRONOUS_API
-		arch_mem_domain_thread_remove(thread);
-#endif
-	}
-
 	LOG_DBG("add thread %p to domain %p\n", thread, domain);
 	sys_dlist_append(&domain->mem_domain_q,
 			 &thread->mem_domain_info.mem_domain_q_node);
@@ -259,7 +248,45 @@ void k_mem_domain_add_thread(struct k_mem_domain *domain, k_tid_t thread)
 #ifdef CONFIG_ARCH_MEM_DOMAIN_SYNCHRONOUS_API
 	arch_mem_domain_thread_add(thread);
 #endif
+}
 
+static void remove_thread_locked(struct k_thread *thread)
+{
+	__ASSERT_NO_MSG(thread != NULL);
+	LOG_DBG("remove thread %p from memory domain %p\n",
+		thread, thread->mem_domain_info.mem_domain);
+	sys_dlist_remove(&thread->mem_domain_info.mem_domain_q_node);
+
+#ifdef CONFIG_ARCH_MEM_DOMAIN_SYNCHRONOUS_API
+	arch_mem_domain_thread_remove(thread);
+#endif
+}
+
+/* Called from thread object initialization */
+void z_mem_domain_init_thread(struct k_thread *thread)
+{
+	k_spinlock_key_t key = k_spin_lock(&lock);
+
+	/* New threads inherit memory domain configuration from parent */
+	add_thread_locked(_current->mem_domain_info.mem_domain, thread);
+	k_spin_unlock(&lock, key);
+}
+
+/* Called when thread aborts during teardown tasks. sched_spinlock is held */
+void z_mem_domain_exit_thread(struct k_thread *thread)
+{
+	k_spinlock_key_t key = k_spin_lock(&lock);
+	remove_thread_locked(thread);
+	k_spin_unlock(&lock, key);
+}
+
+void k_mem_domain_add_thread(struct k_mem_domain *domain, k_tid_t thread)
+{
+	k_spinlock_key_t key;
+
+	key = k_spin_lock(&lock);
+	remove_thread_locked(thread);
+	add_thread_locked(domain, thread);
 	k_spin_unlock(&lock, key);
 }
 

kernel/sched.c

@@ -585,6 +585,12 @@ void z_thread_single_abort(struct k_thread *thread)
 		z_thread_monitor_exit(thread);
 
 #ifdef CONFIG_USERSPACE
+		/* Remove this thread from its memory domain, which takes
+		 * it off the domain's thread list and possibly also arch-
+		 * specific tasks.
+		 */
+		z_mem_domain_exit_thread(thread);
+
 		/* Revoke permissions on thread's ID so that it may be
 		 * recycled
 		 */

kernel/thread.c

@@ -616,10 +616,7 @@ char *z_setup_new_thread(struct k_thread *new_thread,
 	}
 #endif
 #ifdef CONFIG_USERSPACE
-	/* New threads inherit any memory domain membership by the parent */
+	z_mem_domain_init_thread(new_thread);
-	new_thread->mem_domain_info.mem_domain = NULL;
-	k_mem_domain_add_thread(_current->mem_domain_info.mem_domain,
-				new_thread);
 
 	if ((options & K_INHERIT_PERMS) != 0U) {
 		z_thread_perms_inherit(_current, new_thread);