Make exit1() wait sysctl(2) `allprocess' loops.
authormvs <mvs@openbsd.org>
Sun, 11 Aug 2024 15:10:53 +0000 (15:10 +0000)
committermvs <mvs@openbsd.org>
Sun, 11 Aug 2024 15:10:53 +0000 (15:10 +0000)
Regardless on wired userland memory, KERN_FILE_BYPID and KERN_FILE_BYUID
`allprocess' loops have netlock provided sleep points, so concurrent
process exit(1) could crash kernel.

The main exit1() problem is that process teardown begins while process
is still linked to `allprocess' list, and current code doesn't allow to
unlink it first. Wait for concurrent sysctl(2) `allprocess' loops
between PS_EXITING bit setting and list unlinking. Both KERN_FILE_BYPID
and KERN_FILE_BYUID loops do PS_EXITING check and won't deal with dying
process. Concurrent exit1() thread will wait loops keeping process
linked to `allprocess' list.

Tested with i386 dpb(1) run.
Stress tests and ok bluhm.

sys/kern/kern_exit.c
sys/kern/kern_fork.c
sys/kern/kern_sysctl.c
sys/sys/proc.h

index 280af8c..a1d1456 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_exit.c,v 1.230 2024/08/06 18:41:20 claudio Exp $ */
+/*     $OpenBSD: kern_exit.c,v 1.231 2024/08/11 15:10:53 mvs Exp $     */
 /*     $NetBSD: kern_exit.c,v 1.39 1996/04/22 01:38:25 christos Exp $  */
 
 /*
@@ -151,6 +151,9 @@ exit1(struct proc *p, int xexit, int xsig, int flags)
                            PS_ISPWAIT);
                        wakeup(pr->ps_pptr);
                }
+
+               /* Wait for concurrent `allprocess' loops */
+               refcnt_finalize(&pr->ps_refcnt, "psdtor");
        }
 
        /* unlink ourselves from the active threads */
index 52a7c2c..e148c81 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_fork.c,v 1.261 2024/08/06 08:44:54 claudio Exp $ */
+/*     $OpenBSD: kern_fork.c,v 1.262 2024/08/11 15:10:53 mvs Exp $     */
 /*     $NetBSD: kern_fork.c,v 1.29 1996/02/09 18:59:34 christos Exp $  */
 
 /*
@@ -178,6 +178,8 @@ thread_new(struct proc *parent, vaddr_t uaddr)
 void
 process_initialize(struct process *pr, struct proc *p)
 {
+       refcnt_init(&pr->ps_refcnt);
+
        /* initialize the thread links */
        pr->ps_mainproc = p;
        TAILQ_INIT(&pr->ps_threads);
index 74b697e..8cb4772 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_sysctl.c,v 1.436 2024/08/08 15:02:36 bluhm Exp $ */
+/*     $OpenBSD: kern_sysctl.c,v 1.437 2024/08/11 15:10:53 mvs Exp $   */
 /*     $NetBSD: kern_sysctl.c,v 1.17 1996/05/20 17:49:05 mrg Exp $     */
 
 /*-
@@ -1686,6 +1686,9 @@ sysctl_file(int *name, u_int namelen, char *where, size_t *sizep,
                                /* not the pid we are looking for */
                                continue;
                        }
+
+                       refcnt_take(&pr->ps_refcnt);
+
                        matched = 1;
                        fdp = pr->ps_fd;
                        if (pr->ps_textvp)
@@ -1702,6 +1705,9 @@ sysctl_file(int *name, u_int namelen, char *where, size_t *sizep,
                                FILLIT(fp, fdp, i, NULL, pr);
                                FRELE(fp, p);
                        }
+
+                       refcnt_rele_wake(&pr->ps_refcnt);
+
                        /* pid is unique, stop searching */
                        if (arg >= 0)
                                break;
@@ -1721,6 +1727,9 @@ sysctl_file(int *name, u_int namelen, char *where, size_t *sizep,
                                /* not the uid we are looking for */
                                continue;
                        }
+
+                       refcnt_take(&pr->ps_refcnt);
+
                        fdp = pr->ps_fd;
                        if (fdp->fd_cdir)
                                FILLIT(NULL, NULL, KERN_FILE_CDIR, fdp->fd_cdir, pr);
@@ -1734,6 +1743,8 @@ sysctl_file(int *name, u_int namelen, char *where, size_t *sizep,
                                FILLIT(fp, fdp, i, NULL, pr);
                                FRELE(fp, p);
                        }
+
+                       refcnt_rele_wake(&pr->ps_refcnt);
                }
                break;
        default:
index 8527bb1..ad025b1 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: proc.h,v 1.367 2024/08/06 08:44:54 claudio Exp $      */
+/*     $OpenBSD: proc.h,v 1.368 2024/08/11 15:10:53 mvs Exp $  */
 /*     $NetBSD: proc.h,v 1.44 1996/04/22 01:23:21 christos Exp $       */
 
 /*-
@@ -142,6 +142,8 @@ struct pinsyscall {
  *     T       itimer_mtx
  */
 struct process {
+       struct refcnt ps_refcnt;
+
        /*
         * ps_mainproc is the original thread in the process.
         * It's only still special for the handling of