Backout sysctl(2) unlocking. Lock order issue was triggered in UVM
authormvs <mvs@openbsd.org>
Thu, 18 May 2023 10:23:19 +0000 (10:23 +0000)
committermvs <mvs@openbsd.org>
Thu, 18 May 2023 10:23:19 +0000 (10:23 +0000)
layer.

sys/kern/kern_sysctl.c
sys/kern/syscalls.master
sys/kern/uipc_domain.c

index 7c19cf3..f63fc7c 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_sysctl.c,v 1.413 2023/05/17 22:12:51 kettenis Exp $      */
+/*     $OpenBSD: kern_sysctl.c,v 1.414 2023/05/18 10:23:19 mvs Exp $   */
 /*     $NetBSD: kern_sysctl.c,v 1.17 1996/05/20 17:49:05 mrg Exp $     */
 
 /*-
@@ -169,7 +169,7 @@ sys_sysctl(struct proc *p, void *v, register_t *retval)
                syscallarg(void *) new;
                syscallarg(size_t) newlen;
        } */ *uap = v;
-       int error, dokernellock = 1, dolock = 1;
+       int error, dolock = 1;
        size_t savelen = 0, oldlen = 0;
        sysctlfn *fn;
        int name[CTL_MAXNAME];
@@ -204,7 +204,6 @@ sys_sysctl(struct proc *p, void *v, register_t *retval)
                break;
        case CTL_NET:
                fn = net_sysctl;
-               dokernellock = 0;
                break;
        case CTL_FS:
                fn = fs_sysctl;
@@ -232,22 +231,19 @@ sys_sysctl(struct proc *p, void *v, register_t *retval)
        if (SCARG(uap, oldlenp) &&
            (error = copyin(SCARG(uap, oldlenp), &oldlen, sizeof(oldlen))))
                return (error);
-       if (dokernellock)
-               KERNEL_LOCK();
        if (SCARG(uap, old) != NULL) {
                if ((error = rw_enter(&sysctl_lock, RW_WRITE|RW_INTR)) != 0)
-                       goto unlock;
+                       return (error);
                if (dolock) {
                        if (atop(oldlen) > uvmexp.wiredmax - uvmexp.wired) {
                                rw_exit_write(&sysctl_lock);
-                               error = ENOMEM;
-                               goto unlock;
+                               return (ENOMEM);
                        }
                        error = uvm_vslock(p, SCARG(uap, old), oldlen,
                            PROT_READ | PROT_WRITE);
                        if (error) {
                                rw_exit_write(&sysctl_lock);
-                               goto unlock;
+                               return (error);
                        }
                }
                savelen = oldlen;
@@ -259,9 +255,6 @@ sys_sysctl(struct proc *p, void *v, register_t *retval)
                        uvm_vsunlock(p, SCARG(uap, old), savelen);
                rw_exit_write(&sysctl_lock);
        }
-unlock:
-       if (dokernellock)
-               KERNEL_UNLOCK();
        if (error)
                return (error);
        if (SCARG(uap, oldlenp))
index 3faf5dd..34bef2b 100644 (file)
@@ -1,4 +1,4 @@
-;      $OpenBSD: syscalls.master,v 1.247 2023/05/04 09:40:36 mvs Exp $
+;      $OpenBSD: syscalls.master,v 1.248 2023/05/18 10:23:19 mvs Exp $
 ;      $NetBSD: syscalls.master,v 1.32 1996/04/23 10:24:21 mycroft Exp $
 
 ;      @(#)syscalls.master     8.2 (Berkeley) 1/13/94
 199    OBSOL           pad_lseek
 200    OBSOL           pad_truncate
 201    OBSOL           pad_ftruncate
-202    STD NOLOCK      { int sys_sysctl(const int *name, u_int namelen, \
+202    STD             { int sys_sysctl(const int *name, u_int namelen, \
                            void *old, size_t *oldlenp, void *new, \
                            size_t newlen); }
 203    STD             { int sys_mlock(const void *addr, size_t len); }
index ba9a69b..78ae090 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uipc_domain.c,v 1.63 2023/05/18 09:59:43 mvs Exp $    */
+/*     $OpenBSD: uipc_domain.c,v 1.64 2023/05/18 10:23:19 mvs Exp $    */
 /*     $NetBSD: uipc_domain.c,v 1.14 1996/02/09 19:00:44 christos Exp $        */
 
 /*
@@ -188,7 +188,7 @@ net_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
 {
        const struct domain *dp;
        const struct protosw *pr;
-       int error, family, protocol;
+       int family, protocol;
 
        /*
         * All sysctl names at this level are nonterminal.
@@ -213,13 +213,9 @@ net_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
                    newp, newlen));
 #endif
 #if NPFLOW > 0
-       if (family == PF_PFLOW) {
-               KERNEL_LOCK();
-               error = pflow_sysctl(name + 1, namelen - 1, oldp, oldlenp,
-                   newp, newlen);
-               KERNEL_UNLOCK();
-               return (error);
-       }
+       if (family == PF_PFLOW)
+               return (pflow_sysctl(name + 1, namelen - 1, oldp, oldlenp,
+                   newp, newlen));
 #endif
 #ifdef PIPEX
        if (family == PF_PIPEX)
@@ -227,13 +223,9 @@ net_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
                    newp, newlen));
 #endif
 #ifdef MPLS
-       if (family == PF_MPLS) {
-               KERNEL_LOCK();
-               error = mpls_sysctl(name + 1, namelen - 1, oldp, oldlenp,
-                   newp, newlen);
-               KERNEL_UNLOCK();
-               return (error);
-       }
+       if (family == PF_MPLS)
+               return (mpls_sysctl(name + 1, namelen - 1, oldp, oldlenp,
+                   newp, newlen));
 #endif
        dp = pffinddomain(family);
        if (dp == NULL)
@@ -243,13 +235,9 @@ net_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
                return (EISDIR);                /* overloaded */
        protocol = name[1];
        for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++)
-               if (pr->pr_protocol == protocol && pr->pr_sysctl) {
-                       KERNEL_LOCK();
-                       error = (*pr->pr_sysctl)(name + 2, namelen - 2,
-                           oldp, oldlenp, newp, newlen);
-                       KERNEL_UNLOCK();
-                       return (error);
-               }
+               if (pr->pr_protocol == protocol && pr->pr_sysctl)
+                       return ((*pr->pr_sysctl)(name + 2, namelen - 2,
+                           oldp, oldlenp, newp, newlen));
        return (ENOPROTOOPT);
 }