The branch stable/13 has been updated by markj:
URL:
https://cgit.FreeBSD.org/src/commit/?id=d2c1d1b2bc47e451a4cc77ca1a2b0d1648343110
commit d2c1d1b2bc47e451a4cc77ca1a2b0d1648343110
Author: Mark Johnston <ma...@freebsd.org>
AuthorDate: 2023-12-09 15:22:06 +0000
Commit: Mark Johnston <ma...@freebsd.org>
CommitDate: 2023-12-28 03:01:26 +0000
kthread: Set *newtdp earlier in kthread_add1()
syzbot reported a single boot-time crash in g_event_procbody(), a page
fault when dereferencing g_event_td. g_event_td is initialized by the
kproc_kthread_add() call which creates the GEOM event thread:
kproc_kthread_add(g_event_procbody, NULL, &g_proc, &g_event_td,
RFHIGHPID, 0, "geom", "g_event");
I believe that the caller of kproc_kthread_add() was preempted after
adding the new thread to the scheduler, and before setting *newtdp,
which is equal to g_event_td. Thus, since the first action of the GEOM
event thread is to lock itself, it ended up dereferencing a NULL
pointer.
Fix the problem simply by initializing *newtdp earlier. I see no harm
in that, and it matches kproc_create1(). The scheduler provides
sufficient synchronization to ensure that the store is visible to the
new thread, wherever it happens to run.
Reported by: syzbot+5397f4d39219b85a9...@syzkaller.appspotmail.com
Reviewed by: kib
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D42986
(cherry picked from commit ae77041e0714627f9ec8045ca9ee2b6ea563138e)
---
sys/kern/kern_kthread.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/sys/kern/kern_kthread.c b/sys/kern/kern_kthread.c
index 6f7fd8b3d555..9cbc74658432 100644
--- a/sys/kern/kern_kthread.c
+++ b/sys/kern/kern_kthread.c
@@ -287,6 +287,13 @@ kthread_add1(void (*func)(void *), void *arg, struct proc
*p,
}
oldtd = FIRST_THREAD_IN_PROC(p);
+ /*
+ * Set the new thread pointer before the thread starts running: *newtdp
+ * could be a pointer that is referenced by "func".
+ */
+ if (newtdp != NULL)
+ *newtdp = newtd;
+
bzero(&newtd->td_startzero,
__rangeof(struct thread, td_startzero, td_endzero));
bcopy(&oldtd->td_startcopy, &newtd->td_startcopy,
@@ -331,8 +338,6 @@ kthread_add1(void (*func)(void *), void *arg, struct proc
*p,
thread_lock(newtd);
sched_add(newtd, SRQ_BORING);
}
- if (newtdp)
- *newtdp = newtd;
return (0);
}
