Revert lock changes inside ifioctl_get()
authorkn <kn@openbsd.org>
Tue, 8 Nov 2022 21:07:33 +0000 (21:07 +0000)
committerkn <kn@openbsd.org>
Tue, 8 Nov 2022 21:07:33 +0000 (21:07 +0000)
WITNESS isn't happy with r1.667 "Push kernel lock into ifioctl_get()", so
revert it (including r1.668 and r1.669 depending on it):

witness: userret: returning with the following locks held:
exclusive kernel_lock &kernel_lock r = 0 (0xffffffff82455f58)
#0  witness_lock+0x311
#1  ifioctl_get+0x2e
#2  sys_ioctl+0x2c4
#3  syscall+0x384
#4  Xsyscall+0x128
panic: witness_warn
Stopped at      db_enter+0x10:  popq    %rbp
    TID    PID    UID     PRFLAGS     PFLAGS  CPU  COMMAND
* 70588  52613      0         0x3          0    4K pfctl

So back to the drawing board while leaving documentation bits (r1.670).
Thanks Hrvoje.

sys/net/if.c

index 9cb801b..297eac6 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if.c,v 1.671 2022/11/08 18:47:58 kn Exp $     */
+/*     $OpenBSD: if.c,v 1.672 2022/11/08 21:07:33 kn Exp $     */
 /*     $NetBSD: if.c,v 1.35 1996/05/07 05:26:04 thorpej Exp $  */
 
 /*
@@ -1980,7 +1980,9 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p)
        case SIOCGIFRDOMAIN:
        case SIOCGIFGROUP:
        case SIOCGIFLLPRIO:
+               KERNEL_LOCK();
                error = ifioctl_get(cmd, data);
+               KERNEL_UNLOCK();
                return (error);
        }
 
@@ -2429,45 +2431,33 @@ ifioctl_get(u_long cmd, caddr_t data)
 
        switch(cmd) {
        case SIOCGIFCONF:
-               KERNEL_LOCK();
                NET_LOCK_SHARED();
                error = ifconf(data);
                NET_UNLOCK_SHARED();
-               KERNEL_UNLOCK();
                return (error);
        case SIOCIFGCLONERS:
                error = if_clone_list((struct if_clonereq *)data);
                return (error);
        case SIOCGIFGMEMB:
-               KERNEL_LOCK();
                NET_LOCK_SHARED();
                error = if_getgroupmembers(data);
                NET_UNLOCK_SHARED();
-               KERNEL_UNLOCK();
                return (error);
        case SIOCGIFGATTR:
-               KERNEL_LOCK();
                NET_LOCK_SHARED();
                error = if_getgroupattribs(data);
                NET_UNLOCK_SHARED();
-               KERNEL_UNLOCK();
                return (error);
        case SIOCGIFGLIST:
-               KERNEL_LOCK();
                NET_LOCK_SHARED();
                error = if_getgrouplist(data);
                NET_UNLOCK_SHARED();
-               KERNEL_UNLOCK();
                return (error);
        }
 
-       KERNEL_LOCK();
-
        ifp = if_unit(ifr->ifr_name);
-       if (ifp == NULL) {
-               KERNEL_UNLOCK();
+       if (ifp == NULL)
                return (ENXIO);
-       }
 
        NET_LOCK_SHARED();
 
@@ -2539,8 +2529,6 @@ ifioctl_get(u_long cmd, caddr_t data)
 
        NET_UNLOCK_SHARED();
 
-       KERNEL_UNLOCK();
-
        if_put(ifp);
 
        return (error);