Index | Thread | Search

From:
Gerhard Roth <gerhard_roth@genua.de>
Subject:
Kernel protection fault in fill_kproc()
To:
"tech@openbsd.org" <tech@openbsd.org>
Cc:
"mpi@openbsd.org" <mpi@openbsd.org>, Carsten Beckmann <carsten_beckmann@genua.de>
Date:
Mon, 11 Aug 2025 10:45:05 +0000

Download raw body.

Thread
About a year ago, the call to uvm_exit() was moved outside of theĀ 
KERNEL_LOCK() in the reaper() by mpi@. Now we observed a kernel
protection fault that results from this change.

In fill_kproc() we read the vmspace pointer (vm) right at the very
beginning of the function:

        struct vmspace *vm = pr->ps_vmspace;

Sometime later, we try to access it:

	/* fixups that can only be done in the kernel */
	if ((pr->ps_flags & PS_ZOMBIE) == 0) {
		if ((pr->ps_flags & PS_EMBRYO) == 0 && vm != NULL)
			ki->p_vm_rssize = vm_resident_count(vm);


In the meantime the process might have exited and the reaper() can free
the vmspace by calling uvm_exit(). After that, the 'vm' pointer in
fill_kproc() points to stale memory. Accessing it will yield a kernel
protection fault.

BTW: only after freeing the vmspace of the process, the PS_ZOMBIE flag
is set by the reaper().

I propose to put the reaper()'s call to uvm_exit() back under the
kernel lock to avoid the fault.

Gerhard


Index: sys/kern/kern_exit.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_exit.c,v
diff -u -p -u -p -r1.252 kern_exit.c
--- sys/kern/kern_exit.c	10 Aug 2025 15:17:57 -0000	1.252
+++ sys/kern/kern_exit.c	11 Aug 2025 10:30:57 -0000
@@ -498,10 +498,15 @@ reaper(void *arg)
 		} else {
 			struct process *pr = p->p_p;
 
-			/* Release the rest of the process's vmspace */
+			/*
+			 * Release the rest of the process's vmspace
+			 * Use the kernel lock to avoid a race with fill_kproc()
+			 * accessing the vmspace while the process isn't yet a
+			 * zombie.
+			 */
+			KERNEL_LOCK();
 			uvm_exit(pr);
 
-			KERNEL_LOCK();
 			if ((pr->ps_flags & PS_NOZOMBIE) == 0) {
 				/* Process is now a true zombie. */
 				atomic_setbits_int(&pr->ps_flags, PS_ZOMBIE);