Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 7 Feb 2025 14:47:07 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 0db4588bbe6e - stable/14 - thread: Simplify sanitizer integration with thread creation
Message-ID:  <202502071447.517El7ag003004@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/14 has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=0db4588bbe6e9eb83a5a824cc35d35e6fb878e26

commit 0db4588bbe6e9eb83a5a824cc35d35e6fb878e26
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-04-22 15:43:17 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2025-02-07 14:46:53 +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
    
    (cherry picked from commit 800da341bc4a35f4b4d82d104b130825d9a42ffa)
---
 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       |  7 ++++---
 6 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index 8f11bc246593..ee58ad42bce6 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -42,7 +42,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>
@@ -1021,19 +1020,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 102ba609cae7..af125ff28dc0 100644
--- a/sys/kern/kern_thread.c
+++ b/sys/kern/kern_thread.c
@@ -797,6 +797,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);
@@ -804,15 +805,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 51bf1f684c79..0edb631d1475 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 c43d00a223f7..e37f0521861a 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -1260,7 +1260,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);
@@ -1273,6 +1272,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 031959c2aff9..f0ec4ac83517 100644
--- a/sys/vm/vm_glue.c
+++ b/sys/vm/vm_glue.c
@@ -391,11 +391,12 @@ vm_thread_dispose(struct thread *td)
 	ks = td->td_kstack;
 	td->td_kstack = 0;
 	td->td_kstack_pages = 0;
-	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);
+	}
 }
 
 /*



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202502071447.517El7ag003004>