jimzah/freebsd-src
1
0
Fork 0
mirror of https://github.com/freebsd/freebsd-src.git synced 2024-12-04 05:58:57 +00:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

kthread: Set *newtdp earlier in kthread_add1()

Browse Source 
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+5397f4d39219b85a9409@syzkaller.appspotmail.com
Reviewed by:	kib
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D42986
This commit is contained in:
Mark Johnston 2023-12-09 10:22:06 -05:00
parent 3b1904d9eb
commit ae77041e07
1 changed files with 7 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
sys/kern/kern_kthread.c
View File

@ -286,6 +286,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,
@ -330,8 +337,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);
}