From: Hans-Jörg Höxer Subject: Re: [EXT] Re: Kernel protection fault in fill_kproc() To: Cc: Gerhard Roth , "mvs@openbsd.org" , Carsten Beckmann , "mark.kettenis@xs4all.nl" Date: Mon, 22 Sep 2025 18:04:50 +0200 Hi, On Thu, Sep 18, 2025 at 06:10:23PM +0200, Martin Pieuchot wrote: > On 12/09/25(Fri) 13:09, cjeker@diehard.n-r-g.com wrote: > > [...] > > I prefer an explicit if (vm == NULL) return; check in uvmspace_free() to > > make this more obvious. Since that's just optics you can decide. > > Here's an updated version with the NULL check updated. > > Gerhard would you like to ok the diff? Or at least confirm it fixes the > bug you originally reported? Thanks! we have tested your diff and we can confirm, that it fixes the bug Gerhard has reported. Thanks! > Index: kern/kern_sysctl.c > =================================================================== > RCS file: /cvs/src/sys/kern/kern_sysctl.c,v > diff -u -p -r1.482 kern_sysctl.c > --- kern/kern_sysctl.c 6 Aug 2025 14:00:33 -0000 1.482 > +++ kern/kern_sysctl.c 18 Sep 2025 16:03:41 -0000 > @@ -2051,11 +2051,17 @@ fill_kproc(struct process *pr, struct ki > { > struct session *s = pr->ps_session; > struct tty *tp; > - struct vmspace *vm = pr->ps_vmspace; > + struct vmspace *vm = NULL; > struct timespec booted, st, ut, utc; > struct tusage tu; > int isthread; > > + /* exiting/zombie process might no longer have VM space. */ > + if ((pr->ps_flags & PS_EXITING) == 0) { > + vm = pr->ps_vmspace; > + uvmspace_addref(vm); > + } > + > isthread = p != NULL; > if (!isthread) { > p = pr->ps_mainproc; /* XXX */ > @@ -2082,7 +2088,7 @@ fill_kproc(struct process *pr, struct ki > } > > /* fixups that can only be done in the kernel */ > - if ((pr->ps_flags & PS_ZOMBIE) == 0) { > + if ((pr->ps_flags & PS_EXITING) == 0) { > if ((pr->ps_flags & PS_EMBRYO) == 0 && vm != NULL) > ki->p_vm_rssize = vm_resident_count(vm); > calctsru(&tu, &ut, &st, NULL); > @@ -2103,13 +2109,15 @@ fill_kproc(struct process *pr, struct ki > #endif > } > > + uvmspace_free(vm); > + > /* get %cpu and schedule state: just one thread or sum of all? */ > if (isthread) { > ki->p_pctcpu = p->p_pctcpu; > ki->p_stat = p->p_stat; > } else { > ki->p_pctcpu = 0; > - ki->p_stat = (pr->ps_flags & PS_ZOMBIE) ? SDEAD : SIDL; > + ki->p_stat = (pr->ps_flags & PS_EXITING) ? SDEAD : SIDL; > TAILQ_FOREACH(p, &pr->ps_threads, p_thr_link) { > ki->p_pctcpu += p->p_pctcpu; > /* find best state: ONPROC > RUN > STOP > SLEEP > .. */ > Index: kern/tty.c > =================================================================== > RCS file: /cvs/src/sys/kern/tty.c,v > diff -u -p -r1.180 tty.c > --- kern/tty.c 12 Jun 2025 20:37:58 -0000 1.180 > +++ kern/tty.c 18 Sep 2025 16:03:41 -0000 > @@ -2198,8 +2198,8 @@ empty: ttyprintf(tp, "empty foreground > if (run2 || pctcpu2 > pctcpu) > goto update_pickpr; > > - /* if p has less cpu or is zombie, then it's worse */ > - if (pctcpu2 < pctcpu || (pr->ps_flags & PS_ZOMBIE)) > + /* if p has less cpu or is exiting, then it's worse */ > + if (pctcpu2 < pctcpu || (pr->ps_flags & PS_EXITING)) > continue; > update_pickpr: > pickpr = pr; > @@ -2209,7 +2209,7 @@ update_pickpr: > > /* Calculate percentage cpu, resident set size. */ > calc_pctcpu = (pctcpu * 10000 + FSCALE / 2) >> FSHIFT; > - if ((pickpr->ps_flags & (PS_EMBRYO | PS_ZOMBIE)) == 0 && > + if ((pickpr->ps_flags & (PS_EMBRYO | PS_EXITING)) == 0 && > pickpr->ps_vmspace != NULL) > rss = vm_resident_count(pickpr->ps_vmspace); > > Index: uvm/uvm_map.c > =================================================================== > RCS file: /cvs/src/sys/uvm/uvm_map.c,v > diff -u -p -r1.346 uvm_map.c > --- uvm/uvm_map.c 11 Sep 2025 17:04:35 -0000 1.346 > +++ uvm/uvm_map.c 18 Sep 2025 16:04:20 -0000 > @@ -3426,6 +3426,9 @@ uvmspace_purge(struct vmspace *vm) > void > uvmspace_free(struct vmspace *vm) > { > + if (vm == NULL) > + return; > + > if (atomic_dec_int_nv(&vm->vm_refcnt) == 0) { > /* > * Sanity check. Kernel threads never end up here and > > -- Dr. Hans-Jörg Höxer Hans-Joerg_Hoexer@genua.de Senior Expert Kryptographie eXtreme Kernel and Crypto Development genua GmbH Domagkstrasse 7, 85551 Kirchheim bei München tel +49 89 991950-0, fax -999, www.genua.eu Geschäftsführer: Matthias Ochs, Marc Tesch Amtsgericht München HRB 98238 genua ist ein Unternehmen der Bundesdruckerei-Gruppe.