For multicast and broadcast packets udp_input() traverses the loop

of all UDP PCBs.  From there it calls udp_sbappend() while holding
the UDP table mutex.  This ends in sorwakeup() where we finally
grab the kernel lock while holding a mutex.  Witness detects this
misuse.
Use the same solution as for PCB notify.  Collect the affected PCBs
in a temporary list.  The list is protected by exclusive net lock.
Reported-by: syzbot+7596cb96fb9f3c9d6f4f@syzkaller.appspotmail.com
OK sashan@

diff --git a/sys/netinet/in_pcb.h b/sys/netinet/in_pcb.h
index 0e749af..36ad80d 100644 (file)
--- a/sys/netinet/in_pcb.h
+++ b/sys/netinet/in_pcb.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: in_pcb.h,v 1.127 2022/03/21 09:12:34 bluhm Exp $      */
+/*     $OpenBSD: in_pcb.h,v 1.128 2022/03/21 23:37:09 bluhm Exp $      */
 /*     $NetBSD: in_pcb.h,v 1.14 1996/02/13 23:42:00 christos Exp $     */
 
 /*
@@ -102,7 +102,7 @@ struct inpcb {
        LIST_ENTRY(inpcb) inp_hash;             /* [t] local and foreign hash */
        LIST_ENTRY(inpcb) inp_lhash;            /* [t] local port hash */
        TAILQ_ENTRY(inpcb) inp_queue;           /* [t] inet PCB queue */
-       SIMPLEQ_ENTRY(inpcb) inp_notify;        /* [N] queue to notify PCB */
+       SIMPLEQ_ENTRY(inpcb) inp_notify;        /* [N] notify or udp append */
        struct    inpcbtable *inp_table;        /* [I] inet queue/hash table */
        union     inpaddru inp_faddru;          /* Foreign address. */
        union     inpaddru inp_laddru;          /* Local address. */

diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c
index cc679a7..a8dc168 100644 (file)
--- a/sys/netinet/udp_usrreq.c
+++ b/sys/netinet/udp_usrreq.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: udp_usrreq.c,v 1.276 2022/03/21 19:39:56 bluhm Exp $  */
+/*     $OpenBSD: udp_usrreq.c,v 1.277 2022/03/21 23:37:09 bluhm Exp $  */
 /*     $NetBSD: udp_usrreq.c,v 1.28 1996/03/16 23:54:03 christos Exp $ */
 
 /*
@@ -342,7 +342,8 @@ udp_input(struct mbuf **mp, int *offp, int proto, int af)
        }
 
        if (m->m_flags & (M_BCAST|M_MCAST)) {
-               struct inpcb *last;
+               SIMPLEQ_HEAD(, inpcb) inpcblist;
+
                /*
                 * Deliver a multicast or broadcast datagram to *all* sockets
                 * for which the local and remote addresses and ports match
@@ -363,8 +364,8 @@ udp_input(struct mbuf **mp, int *offp, int proto, int af)
                 * Locate pcb(s) for datagram.
                 * (Algorithm copied from raw_intr().)
                 */
-               last = NULL;
-               NET_ASSERT_LOCKED();
+               NET_ASSERT_WLOCKED();
+               SIMPLEQ_INIT(&inpcblist);
                mtx_enter(&udbtable.inpt_mtx);
                TAILQ_FOREACH(inp, &udbtable.inpt_queue, inp_queue) {
                        if (inp->inp_socket->so_state & SS_CANTRCVMORE)
@@ -419,16 +420,9 @@ udp_input(struct mbuf **mp, int *offp, int proto, int af)
                                        continue;
                        }
 
-                       if (last != NULL) {
-                               struct mbuf *n;
+                       in_pcbref(inp);
+                       SIMPLEQ_INSERT_TAIL(&inpcblist, inp, inp_notify);
 
-                               n = m_copym(m, 0, M_COPYALL, M_NOWAIT);
-                               if (n != NULL) {
-                                       udp_sbappend(last, n, ip, ip6, iphlen,
-                                           uh, &srcsa.sa, 0);
-                               }
-                       }
-                       last = inp;
                        /*
                         * Don't look for additional matches if this one does
                         * not have either the SO_REUSEPORT or SO_REUSEADDR
@@ -437,13 +431,13 @@ udp_input(struct mbuf **mp, int *offp, int proto, int af)
                         * port.  It assumes that an application will never
                         * clear these options after setting them.
                         */
-                       if ((last->inp_socket->so_options & (SO_REUSEPORT |
+                       if ((inp->inp_socket->so_options & (SO_REUSEPORT |
                            SO_REUSEADDR)) == 0)
                                break;
                }
                mtx_leave(&udbtable.inpt_mtx);
 
-               if (last == NULL) {
+               if (SIMPLEQ_EMPTY(&inpcblist)) {
                        /*
                         * No matching pcb found; discard datagram.
                         * (No need to send an ICMP Port Unreachable
@@ -453,7 +447,20 @@ udp_input(struct mbuf **mp, int *offp, int proto, int af)
                        goto bad;
                }
 
-               udp_sbappend(last, m, ip, ip6, iphlen, uh, &srcsa.sa, 0);
+               while ((inp = SIMPLEQ_FIRST(&inpcblist)) != NULL) {
+                       struct mbuf *n;
+
+                       SIMPLEQ_REMOVE_HEAD(&inpcblist, inp_notify);
+                       if (SIMPLEQ_EMPTY(&inpcblist))
+                               n = m;
+                       else
+                               n = m_copym(m, 0, M_COPYALL, M_NOWAIT);
+                       if (n != NULL) {
+                               udp_sbappend(inp, n, ip, ip6, iphlen, uh,
+                                   &srcsa.sa, 0);
+                       }
+                       in_pcbunref(inp);
+               }
                return IPPROTO_DONE;
        }
        /*