From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: Kernel protection fault in fill_kproc()
To: Dave Voutila <dv@sisu.io>
Cc: Gerhard Roth <gerhard_roth@genua.de>, "tech@openbsd.org" <tech@openbsd.org>, "mpi@openbsd.org" <mpi@openbsd.org>, Carsten Beckmann <carsten_beckmann@genua.de>
Date: Mon, 11 Aug 2025 17:57:23 +0300

On Mon, Aug 11, 2025 at 10:34:40AM -0400, Dave Voutila wrote:
> Gerhard Roth <gerhard_roth@genua.de> writes:
> 
> > 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.
> 
> I don't think this is the correct approach.
> 
> I don't tend to work in this area, but this looks possibly related to
> unlocking in sysctl given fill_kproc() is seeing the memory issues. A
> lot has changed in kern_sysctl.c in the past few months.
> 

fill_kproc() and the whole sysctl_doproc() are still locked as they was.
The most unlocking work was done in the network stack related sysctl(2)
paths. Please try to look at the code before blame someone.

> >
> > 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);
>