jcs@ has reported the following panic which started appearing as
a consequence of the recent file close unlocking:

panic: kernel diagnostic assertion "timo || _kernel_lock_held()" failed: file 
"/usr/src/sys/kern/kern_synch.c", line 123

panic
__assert
tsleep
usbd_transfer
usbd_do_request_flags
ure_iff
ure_ioctl
in6_delmulti
in6_leavegroup
in6_freemoptions
in_pcbdetach
udp_detach
soclose

The above shows that soclose() can call parts of the kernel that still
need the kernel lock. This is now apparent because close(2) is unlocked.
However, the issue has been present longer. If a thread raced with the
closing of a socket file descriptor, the final FRELE() might be invoked
in an unlocked context, triggering a similar situation.

I think most of the socket handling is now under NET_LOCK(). However,
calls to the driver layer from the socket code need to be serialized
with the kernel lock. The following diff does that for SIOCDELMULTI
ioctl requests that can be invoked from the socket close path. I wonder
if this is actually enough and if most of soclose() should be put back
under KERNEL_LOCK() as a precaution...

Index: netinet/in.c
===================================================================
RCS file: src/sys/netinet/in.c,v
retrieving revision 1.168
diff -u -p -r1.168 in.c
--- netinet/in.c        1 Dec 2019 21:12:42 -0000       1.168
+++ netinet/in.c        11 Mar 2020 16:26:44 -0000
@@ -929,7 +929,9 @@ in_delmulti(struct in_multi *inm)
                            sizeof(struct sockaddr_in);
                        satosin(&ifr.ifr_addr)->sin_family = AF_INET;
                        satosin(&ifr.ifr_addr)->sin_addr = inm->inm_addr;
+                       KERNEL_LOCK();
                        (*ifp->if_ioctl)(ifp, SIOCDELMULTI, (caddr_t)&ifr);
+                       KERNEL_UNLOCK();
 
                        TAILQ_REMOVE(&ifp->if_maddrlist, &inm->inm_ifma,
                            ifma_list);
Index: netinet/ip_mroute.c
===================================================================
RCS file: src/sys/netinet/ip_mroute.c,v
retrieving revision 1.128
diff -u -p -r1.128 ip_mroute.c
--- netinet/ip_mroute.c 2 Sep 2019 13:12:09 -0000       1.128
+++ netinet/ip_mroute.c 11 Mar 2020 16:26:44 -0000
@@ -772,7 +772,9 @@ vif_delete(struct ifnet *ifp)
        satosin(&ifr.ifr_addr)->sin_len = sizeof(struct sockaddr_in);
        satosin(&ifr.ifr_addr)->sin_family = AF_INET;
        satosin(&ifr.ifr_addr)->sin_addr = zeroin_addr;
+       KERNEL_LOCK();
        (*ifp->if_ioctl)(ifp, SIOCDELMULTI, (caddr_t)&ifr);
+       KERNEL_UNLOCK();
 
        free(v, M_MRTABLE, sizeof(*v));
 }
Index: netinet6/in6.c
===================================================================
RCS file: src/sys/netinet6/in6.c,v
retrieving revision 1.234
diff -u -p -r1.234 in6.c
--- netinet6/in6.c      18 Nov 2019 22:08:59 -0000      1.234
+++ netinet6/in6.c      11 Mar 2020 16:26:44 -0000
@@ -1100,7 +1100,9 @@ in6_delmulti(struct in6_multi *in6m)
                        ifr.ifr_addr.sin6_len = sizeof(struct sockaddr_in6);
                        ifr.ifr_addr.sin6_family = AF_INET6;
                        ifr.ifr_addr.sin6_addr = in6m->in6m_addr;
+                       KERNEL_LOCK();
                        (*ifp->if_ioctl)(ifp, SIOCDELMULTI, (caddr_t)&ifr);
+                       KERNEL_UNLOCK();
 
                        TAILQ_REMOVE(&ifp->if_maddrlist, &in6m->in6m_ifma,
                            ifma_list);
Index: netinet6/ip6_mroute.c
===================================================================
RCS file: src/sys/netinet6/ip6_mroute.c,v
retrieving revision 1.122
diff -u -p -r1.122 ip6_mroute.c
--- netinet6/ip6_mroute.c       4 Sep 2019 16:13:49 -0000       1.122
+++ netinet6/ip6_mroute.c       11 Mar 2020 16:26:44 -0000
@@ -565,7 +565,9 @@ ip6_mrouter_detach(struct ifnet *ifp)
        memset(&ifr, 0, sizeof(ifr));
        ifr.ifr_addr.sin6_family = AF_INET6;
        ifr.ifr_addr.sin6_addr = in6addr_any;
+       KERNEL_LOCK();
        (*ifp->if_ioctl)(ifp, SIOCDELMULTI, (caddr_t)&ifr);
+       KERNEL_UNLOCK();
 
        free(m6, M_MRTABLE, sizeof(*m6));
 }

Reply via email to