From 63abc0ec39b50ac5d2ba006ddc60fcccd6f0d151 Mon Sep 17 00:00:00 2001 From: bluhm Date: Mon, 21 Mar 2022 23:37:09 +0000 Subject: [PATCH] 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@ --- sys/netinet/in_pcb.h | 4 ++-- sys/netinet/udp_usrreq.c | 39 +++++++++++++++++++++++---------------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/sys/netinet/in_pcb.h b/sys/netinet/in_pcb.h index 0e749af9c3b..36ad80d8fbb 100644 --- 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 cc679a7adc4..a8dc16826ae 100644 --- 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; } /* -- 2.20.1