git: 800da341bc4a - main - thread: Simplify sanitizer integration with thread creation

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Mon, 22 Apr 2024 15:54:27 UTC
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=800da341bc4a35f4b4d82d104b130825d9a42ffa

commit 800da341bc4a35f4b4d82d104b130825d9a42ffa
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-04-22 15:43:17 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-04-22 15:46:59 +0000

    thread: Simplify sanitizer integration with thread creation
    
    fork() may allocate a new thread in one of two ways: from UMA, or cached
    in a freed proc that was just allocated from UMA.  In either case, KASAN
    and KMSAN need to initialize some state; in particular they need to
    initialize the shadow mapping of the new thread's stack.
    
    This is done differently between KASAN and KMSAN, which is confusing.
    This patch improves things a bit:
    - Add a new thread_recycle() function, which moves all kernel stack
      handling out of kern_fork.c, since it doesn't really belong there.
    - Then, thread_alloc_stack() has only one local caller, so just inline
      it.
    - Avoid redundant shadow stack initialization: thread_alloc()
      initializes the KMSAN shadow stack (via kmsan_thread_alloc()) even
      through vm_thread_new() already did that.
    - Add kasan_thread_alloc(), for consistency with kmsan_thread_alloc().
    
    No functional change intended.
    
    Reviewed by:    khng
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D44891
---
 sys/kern/kern_fork.c   | 17 +++--------------
 sys/kern/kern_thread.c | 20 ++++++++++++--------
 sys/kern/subr_asan.c   | 10 ++++++++++
 sys/sys/asan.h         |  4 ++++
 sys/sys/proc.h         |  2 +-
 sys/vm/vm_glue.c       |  9 ++++-----
 6 files changed, 34 insertions(+), 28 deletions(-)

diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index d0044dfc19a0..41f6a76c4fa1 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -40,7 +40,6 @@
 
 #include <sys/param.h>
 #include <sys/systm.h>
-#include <sys/asan.h>
 #include <sys/bitstring.h>
 #include <sys/sysproto.h>
 #include <sys/eventhandler.h>
@@ -1019,19 +1018,9 @@ fork1(struct thread *td, struct fork_req *fr)
 		}
 		proc_linkup(newproc, td2);
 	} else {
-		kmsan_thread_alloc(td2);
-		if (td2->td_kstack == 0 || td2->td_kstack_pages != pages) {
-			if (td2->td_kstack != 0)
-				vm_thread_dispose(td2);
-			if (!thread_alloc_stack(td2, pages)) {
-				error = ENOMEM;
-				goto fail2;
-			}
-		} else {
-			kasan_mark((void *)td2->td_kstack,
-			    ptoa(td2->td_kstack_pages),
-			    ptoa(td2->td_kstack_pages), 0);
-		}
+		error = thread_recycle(td2, pages);
+		if (error != 0)
+			goto fail2;
 	}
 
 	if ((flags & RFMEM) == 0) {
diff --git a/sys/kern/kern_thread.c b/sys/kern/kern_thread.c
index 0bcd5c2f2add..725791769ac3 100644
--- a/sys/kern/kern_thread.c
+++ b/sys/kern/kern_thread.c
@@ -798,6 +798,7 @@ thread_alloc(int pages)
 	}
 	td->td_tid = tid;
 	bzero(&td->td_sa.args, sizeof(td->td_sa.args));
+	kasan_thread_alloc(td);
 	kmsan_thread_alloc(td);
 	cpu_thread_alloc(td);
 	EVENTHANDLER_DIRECT_INVOKE(thread_ctor, td);
@@ -805,15 +806,18 @@ thread_alloc(int pages)
 }
 
 int
-thread_alloc_stack(struct thread *td, int pages)
+thread_recycle(struct thread *td, int pages)
 {
-
-	KASSERT(td->td_kstack == 0,
-	    ("thread_alloc_stack called on a thread with kstack"));
-	if (!vm_thread_new(td, pages))
-		return (0);
-	cpu_thread_alloc(td);
-	return (1);
+	if (td->td_kstack == 0 || td->td_kstack_pages != pages) {
+		if (td->td_kstack != 0)
+			vm_thread_dispose(td);
+		if (!vm_thread_new(td, pages))
+			return (ENOMEM);
+		cpu_thread_alloc(td);
+	}
+	kasan_thread_alloc(td);
+	kmsan_thread_alloc(td);
+	return (0);
 }
 
 /*
diff --git a/sys/kern/subr_asan.c b/sys/kern/subr_asan.c
index c3664d9cf6c2..75d9e75c531a 100644
--- a/sys/kern/subr_asan.c
+++ b/sys/kern/subr_asan.c
@@ -39,6 +39,7 @@ __KERNEL_RCSID(0, "$NetBSD: subr_asan.c,v 1.26 2020/09/10 14:10:46 maxv Exp $");
 #include <sys/systm.h>
 #include <sys/asan.h>
 #include <sys/kernel.h>
+#include <sys/proc.h>
 #include <sys/stack.h>
 #include <sys/sysctl.h>
 
@@ -294,6 +295,15 @@ kasan_mark(const void *addr, size_t size, size_t redzsize, uint8_t code)
 	}
 }
 
+void
+kasan_thread_alloc(struct thread *td)
+{
+	if (td->td_kstack != 0) {
+		kasan_mark((void *)td->td_kstack, ptoa(td->td_kstack_pages),
+		    ptoa(td->td_kstack_pages), 0);
+	}
+}
+
 /* -------------------------------------------------------------------------- */
 
 #define ADDR_CROSSES_SCALE_BOUNDARY(addr, size) 		\
diff --git a/sys/sys/asan.h b/sys/sys/asan.h
index a3becdef5f57..6a01d0531725 100644
--- a/sys/sys/asan.h
+++ b/sys/sys/asan.h
@@ -53,14 +53,18 @@
 #define	KASAN_KSTACK_FREED	0xFE
 #define	KASAN_EXEC_ARGS_FREED	0xFF
 
+struct thread;
+
 void kasan_init(void);
 void kasan_init_early(vm_offset_t, size_t);
 void kasan_shadow_map(vm_offset_t, size_t);
 void kasan_mark(const void *, size_t, size_t, uint8_t);
+void kasan_thread_alloc(struct thread *);
 #else /* KASAN */
 #define kasan_init()
 #define kasan_shadow_map(a, s)
 #define kasan_mark(p, s, l, c)
+#define kasan_thread_alloc(t)
 #endif /* !KASAN */
 
 #endif /* !_SYS_ASAN_H_ */
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 33b836f4150e..1b542d1374b4 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -1262,7 +1262,6 @@ void	cpu_thread_free(struct thread *);
 void	cpu_thread_swapin(struct thread *);
 void	cpu_thread_swapout(struct thread *);
 struct	thread *thread_alloc(int pages);
-int	thread_alloc_stack(struct thread *, int pages);
 int	thread_check_susp(struct thread *td, bool sleep);
 void	thread_cow_get_proc(struct thread *newtd, struct proc *p);
 void	thread_cow_get(struct thread *newtd, struct thread *td);
@@ -1275,6 +1274,7 @@ void	thread_exit(void) __dead2;
 void	thread_free(struct thread *td);
 void	thread_link(struct thread *td, struct proc *p);
 void	thread_reap_barrier(void);
+int	thread_recycle(struct thread *, int pages);
 int	thread_single(struct proc *p, int how);
 void	thread_single_end(struct proc *p, int how);
 void	thread_stash(struct thread *td);
diff --git a/sys/vm/vm_glue.c b/sys/vm/vm_glue.c
index 4292a7533503..f9235fe03fab 100644
--- a/sys/vm/vm_glue.c
+++ b/sys/vm/vm_glue.c
@@ -543,8 +543,6 @@ vm_thread_new(struct thread *td, int pages)
 	td->td_kstack = ks;
 	td->td_kstack_pages = pages;
 	td->td_kstack_domain = ks_domain;
-	kasan_mark((void *)ks, ptoa(pages), ptoa(pages), 0);
-	kmsan_mark((void *)ks, ptoa(pages), KMSAN_STATE_UNINIT);
 	return (1);
 }
 
@@ -562,11 +560,12 @@ vm_thread_dispose(struct thread *td)
 	td->td_kstack = 0;
 	td->td_kstack_pages = 0;
 	td->td_kstack_domain = MAXMEMDOM;
-	kasan_mark((void *)ks, 0, ptoa(pages), KASAN_KSTACK_FREED);
-	if (pages == kstack_pages)
+	if (pages == kstack_pages) {
+		kasan_mark((void *)ks, 0, ptoa(pages), KASAN_KSTACK_FREED);
 		uma_zfree(kstack_cache, (void *)ks);
-	else
+	} else {
 		vm_thread_stack_dispose(ks, pages);
+	}
 }
 
 /*