git: 71867653008c - main - udp: improve handling of cached route

The branch main has been updated by tuexen:

URL: https://cgit.FreeBSD.org/src/commit/?id=71867653008ce17a66a9c935e9dc29c1320bf48b

commit 71867653008ce17a66a9c935e9dc29c1320bf48b
Author:     Michael Tuexen <tuexen@FreeBSD.org>
AuthorDate: 2024-07-28 21:25:22 +0000
Commit:     Michael Tuexen <tuexen@FreeBSD.org>
CommitDate: 2024-07-28 21:36:48 +0000

    udp: improve handling of cached route
    
    The inp_route pointer should only be provided to the network
    layer, when no destination address is provided. This is only
    one of the conditions, where a write lock is needed.
    If, for example, the route is also cached, when the socket is
    unbound, problems show up, when the sendto is called, then
    connect and finally send, when the route for the addresses
    provided in the sendto and connect call use different outgoing
    interfaces.
    While there, clearly document why the write lock is taken.
    
    Reported by:            syzbot+59122d2e848087d3355a@syzkaller.appspotmail.com
    Reviewed by:            Peter Lei, glebius
    MFC after:              3 days
    Sponsored by:           Netflix, Inc.
    Differential Revision:  https://reviews.freebsd.org/D46056
---
 sys/netinet/udp_usrreq.c   | 20 ++++++++++++--------
 sys/netinet6/udp6_usrreq.c |  2 +-
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c
index 9dad79e95b04..1666d4bd3142 100644
--- a/sys/netinet/udp_usrreq.c
+++ b/sys/netinet/udp_usrreq.c
@@ -1087,7 +1087,6 @@ udp_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr,
 	uint16_t cscov = 0;
 	uint32_t flowid = 0;
 	uint8_t flowtype = M_HASHTYPE_NONE;
-	bool use_cached_route;
 
 	inp = sotoinpcb(so);
 	KASSERT(inp != NULL, ("udp_send: inp == NULL"));
@@ -1116,16 +1115,21 @@ udp_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr,
 	sin = (struct sockaddr_in *)addr;
 
 	/*
-	 * udp_send() may need to temporarily bind or connect the current
-	 * inpcb.  As such, we don't know up front whether we will need the
-	 * pcbinfo lock or not.  Do any work to decide what is needed up
-	 * front before acquiring any locks.
+	 * In the following cases we want a write lock on the inp for either
+	 * local operations or for possible route cache updates in the IPv6
+	 * output path:
+	 * - on connected sockets (sin6 is NULL) for route cache updates,
+	 * - when we are not bound to an address and source port (it is
+	 *   in_pcbinshash() which will require the write lock).
+	 * - when we are using a mapped address on an IPv6 socket (for
+	 *   updating inp_vflag).
 	 *
 	 * We will need network epoch in either case, to safely lookup into
 	 * pcb hash.
 	 */
-	use_cached_route = sin == NULL || (inp->inp_laddr.s_addr == INADDR_ANY && inp->inp_lport == 0);
-	if (use_cached_route || (flags & PRUS_IPV6) != 0)
+	if (sin == NULL ||
+	    (inp->inp_laddr.s_addr == INADDR_ANY && inp->inp_lport == 0) ||
+	    (flags & PRUS_IPV6) != 0)
 		INP_WLOCK(inp);
 	else
 		INP_RLOCK(inp);
@@ -1476,7 +1480,7 @@ udp_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr,
 	else
 		UDP_PROBE(send, NULL, inp, &ui->ui_i, inp, &ui->ui_u);
 	error = ip_output(m, inp->inp_options,
-	    use_cached_route ? &inp->inp_route : NULL, ipflags,
+	    sin == NULL ? &inp->inp_route : NULL, ipflags,
 	    inp->inp_moptions, inp);
 	INP_UNLOCK(inp);
 	NET_EPOCH_EXIT(et);
diff --git a/sys/netinet6/udp6_usrreq.c b/sys/netinet6/udp6_usrreq.c
index 414be35b5bef..d7d85c6b3d13 100644
--- a/sys/netinet6/udp6_usrreq.c
+++ b/sys/netinet6/udp6_usrreq.c
@@ -937,7 +937,7 @@ udp6_send(struct socket *so, int flags_arg, struct mbuf *m,
 	else
 		UDP_PROBE(send, NULL, inp, ip6, inp, udp6);
 	error = ip6_output(m, optp,
-	    INP_WLOCKED(inp) ? &inp->inp_route6 : NULL, flags,
+	    sin6 == NULL ? &inp->inp_route6 : NULL, flags,
 	    inp->in6p_moptions, NULL, inp);
 	INP_UNLOCK(inp);
 	NET_EPOCH_EXIT(et);