Backout priterator() for walking allprocess list.
authorbluhm <bluhm@openbsd.org>
Fri, 19 Jan 2024 01:43:26 +0000 (01:43 +0000)
committerbluhm <bluhm@openbsd.org>
Fri, 19 Jan 2024 01:43:26 +0000 (01:43 +0000)
This approach does not work as LIST_NEXT() of a removed element
does not return NULL.  I causes a crash in syzcaller and triggers
kernel diagnostic assertion "vp->v_uvcount == 0" in sys/kern/kern_unveil.c
line 845 during reboot.  Unfortunately the backout brings back the
race in fill_file() and fstat(1) may crash the kernel.

Reported-by: syzbot+54fba1c004d7383d5e85@syzkaller.appspotmail.com
sys/kern/kern_exit.c
sys/kern/kern_fork.c
sys/kern/kern_proc.c
sys/kern/kern_sysctl.c
sys/sys/proc.h

index 8221cff..f881ff1 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_exit.c,v 1.219 2024/01/16 19:05:01 deraadt Exp $ */
+/*     $OpenBSD: kern_exit.c,v 1.220 2024/01/19 01:43:26 bluhm Exp $   */
 /*     $NetBSD: kern_exit.c,v 1.39 1996/04/22 01:38:25 christos Exp $  */
 
 /*
@@ -165,8 +165,6 @@ exit1(struct proc *p, int xexit, int xsig, int flags)
                /* main thread gotta wait because it has the pid, et al */
                while (pr->ps_threadcnt > 1)
                        tsleep_nsec(&pr->ps_threads, PWAIT, "thrdeath", INFSLP);
-               LIST_REMOVE(pr, ps_list);
-               refcnt_finalize(&pr->ps_refcnt, "psdtor");
        }
 
        rup = pr->ps_ru;
@@ -259,6 +257,7 @@ exit1(struct proc *p, int xexit, int xsig, int flags)
 
        if ((p->p_flag & P_THREAD) == 0) {
                LIST_REMOVE(pr, ps_hash);
+               LIST_REMOVE(pr, ps_list);
 
                if ((pr->ps_flags & PS_NOZOMBIE) == 0)
                        LIST_INSERT_HEAD(&zombprocess, pr, ps_list);
index 239873b..b7b93d8 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_fork.c,v 1.255 2024/01/16 19:05:01 deraadt Exp $ */
+/*     $OpenBSD: kern_fork.c,v 1.256 2024/01/19 01:43:26 bluhm Exp $   */
 /*     $NetBSD: kern_fork.c,v 1.29 1996/02/09 18:59:34 christos Exp $  */
 
 /*
@@ -197,7 +197,6 @@ process_initialize(struct process *pr, struct proc *p)
        LIST_INIT(&pr->ps_sigiolst);
        TAILQ_INIT(&pr->ps_tslpqueue);
 
-       refcnt_init(&pr->ps_refcnt);
        rw_init(&pr->ps_lock, "pslock");
        mtx_init(&pr->ps_mtx, IPL_HIGH);
 
index 66a8a4d..165d10c 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_proc.c,v 1.96 2024/01/15 15:47:37 mvs Exp $      */
+/*     $OpenBSD: kern_proc.c,v 1.97 2024/01/19 01:43:27 bluhm Exp $    */
 /*     $NetBSD: kern_proc.c,v 1.14 1996/02/09 18:59:41 christos Exp $  */
 
 /*
@@ -231,26 +231,6 @@ prfind(pid_t pid)
        return (NULL);
 }
 
-struct process *
-priterator(struct process *ps)
-{
-       struct process *nps;
-
-       KERNEL_ASSERT_LOCKED();
-
-       if (ps == NULL)
-               nps = LIST_FIRST(&allprocess);
-       else
-               nps = LIST_NEXT(ps, ps_list);
-
-       if (nps)
-               refcnt_take(&nps->ps_refcnt);
-       if (ps)
-               refcnt_rele_wake(&ps->ps_refcnt);
-
-       return nps;
-}
-
 /*
  * Locate a process group by number
  */
index 784ffae..862efaf 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_sysctl.c,v 1.423 2024/01/18 08:48:32 mvs Exp $   */
+/*     $OpenBSD: kern_sysctl.c,v 1.424 2024/01/19 01:43:27 bluhm Exp $ */
 /*     $NetBSD: kern_sysctl.c,v 1.17 1996/05/20 17:49:05 mrg Exp $     */
 
 /*-
@@ -1529,7 +1529,7 @@ sysctl_file(int *name, u_int namelen, char *where, size_t *sizep,
                        break;
                }
                matched = 0;
-               for (pr = priterator(NULL); pr != NULL; pr = priterator(pr)) {
+               LIST_FOREACH(pr, &allprocess, ps_list) {
                        /*
                         * skip system, exiting, embryonic and undead
                         * processes
@@ -1561,7 +1561,7 @@ sysctl_file(int *name, u_int namelen, char *where, size_t *sizep,
                        error = ESRCH;
                break;
        case KERN_FILE_BYUID:
-               for (pr = priterator(NULL); pr != NULL; pr = priterator(pr)) {
+               LIST_FOREACH(pr, &allprocess, ps_list) {
                        /*
                         * skip system, exiting, embryonic and undead
                         * processes
index 96efd27..576e697 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: proc.h,v 1.354 2024/01/16 19:05:00 deraadt Exp $      */
+/*     $OpenBSD: proc.h,v 1.355 2024/01/19 01:43:27 bluhm Exp $        */
 /*     $NetBSD: proc.h,v 1.44 1996/04/22 01:23:21 christos Exp $       */
 
 /*-
@@ -50,7 +50,6 @@
 #include <sys/resource.h>              /* For struct rusage */
 #include <sys/rwlock.h>                        /* For struct rwlock */
 #include <sys/sigio.h>                 /* For struct sigio */
-#include <sys/refcnt.h>
 
 #ifdef _KERNEL
 #include <sys/atomic.h>
@@ -172,7 +171,6 @@ struct process {
 
        struct  futex_list ps_ftlist;   /* futexes attached to this process */
        struct  tslpqueue ps_tslpqueue; /* [p] queue of threads in thrsleep */
-       struct  refcnt  ps_refcnt;
        struct  rwlock  ps_lock;        /* per-process rwlock */
        struct  mutex   ps_mtx;         /* per-process mutex */
 
@@ -540,7 +538,6 @@ void        freepid(pid_t);
 
 struct process *prfind(pid_t); /* Find process by id. */
 struct process *zombiefind(pid_t); /* Find zombie process by id. */
-struct process *priterator(struct process *);
 struct proc *tfind(pid_t);     /* Find thread by id. */
 struct pgrp *pgfind(pid_t);    /* Find process group by id. */
 struct proc *tfind_user(pid_t, struct process *);