From nobody Sat Feb 22 02:12:48 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Z09WP0p69z5nqjB; Sat, 22 Feb 2025 02:12:49 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Z09WN5j1wz3hZm; Sat, 22 Feb 2025 02:12:48 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1740190368; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=PcBsLradsdb8sGwBcbeSWlbhM+DWAzjCTAK5q8diwwk=; b=UxjuWMShjmgkX3psuej/Fml1S/M63A4+ZelzrFrpkOR4+Dh0TGGzgPn5qagvLsATruDEem lvf3uDLNQJWf1qWu88pd3DEKF+JYR8FuSoWrlgbwUwPCzxDUzgiP0MkPGs2Mgo9eCM2PZu 8mm2ejaXo/anAs6rUo6AirutEFKOyBmQHl3PDA51J+zRNvsivvLQAjgJxwmr86+FVfJ3g7 yDH9f0vmVpBHAFnK+94sA99lUhMGRpcmXiNls4UxukGkPGYOVfCFHmYjOTdAOWnMnB3nX7 zup8lXzYDJNLYFz9Gq1GMKXACSjynlcysp0h84UeBZK2f2CySksuTrYlKX3feA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1740190368; a=rsa-sha256; cv=none; b=n6NyAAD5+PZ5GT0jtvMZgkrsMe1HH9RyfWkjbJRGE9tXi8X0Xgz3/zttz8e6l7PQbVj6gR FVh83C/DeHQ0qTpNomlrZcYKfbblk//BKgFxB1Tu3fo98XvP2axwVmT05ptxVXotsWSnQo /7m7BW9MfyZUNxsg0W5dSD661ia864R6zuQ65fgEAIrUm4eK7MUw4OwSn5hiRpF9IeOzdF SGIFqnyZE51dHViZy8/f2K1qZ92lOzPAQH9ZP8fJiWRiI8JvB+SseIchAE4MW+4Cz3nX1r 6rYcgkl2MeKPm2RxZ3mhBYoRTQaXUycz30foU9lF92T6XWMiTh2/9GMiskFrRw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1740190368; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=PcBsLradsdb8sGwBcbeSWlbhM+DWAzjCTAK5q8diwwk=; b=R4xsbQZkxT2jLHH2jNz+7gUFC39gOHqqxnla7o5C1/HTKCDM/WURr9uCZWnB+JIkfQLW1M Y0KacrmUDUahM2ty87O4aTXGFaEthAAzCJ0N8CxF5axJ0Yk2Qq9hKB1G27ORjUIOCoVINP 4GDGlSsAcXdIqGtIQnxiRep+ublSUu100y0rpZOKmkPz4rGJeCzqpSralE1KefK2FBvyvw /Gmv5GxNWC4NZYxJv7OkpFFk+C1pguqLTCnnV0AJp4GGaSpS7PX5HeoB3dVdbDrHXDHzdr jk00QBghP9EMoIfGpHbT6TeKb39HaNKn3G4/9a61VTKrF40XBvi3BjqjsEcczw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4Z09WN4Gqdzpbr; Sat, 22 Feb 2025 02:12:48 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 51M2CmIw099362; Sat, 22 Feb 2025 02:12:48 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 51M2CmsU099359; Sat, 22 Feb 2025 02:12:48 GMT (envelope-from git) Date: Sat, 22 Feb 2025 02:12:48 GMT Message-Id: <202502220212.51M2CmsU099359@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Gleb Smirnoff Subject: git: 69c05f428714 - main - udp: make sendto(2) on unconnected UDP socket use public inpcb KPIs List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: glebius X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 69c05f42871406b4b2b2dac00a268d1da0cacd3e Auto-Submitted: auto-generated The branch main has been updated by glebius: URL: https://cgit.FreeBSD.org/src/commit/?id=69c05f42871406b4b2b2dac00a268d1da0cacd3e commit 69c05f42871406b4b2b2dac00a268d1da0cacd3e Author: Gleb Smirnoff AuthorDate: 2025-02-22 02:11:17 +0000 Commit: Gleb Smirnoff CommitDate: 2025-02-22 02:11:17 +0000 udp: make sendto(2) on unconnected UDP socket use public inpcb KPIs UDP allows to sendto(2) on unconnected socket. The original BSD devise was that such action would create a temporary (for the duration of the syscall) connection between our inpcb and remote addr:port specified in sockaddr 'to' of the syscall. This devise was broken in 2002 in 90162a4e87f0. For more motivation on the removal of the temporary connection see email [1]. Since the removal of the true temporary connection the sendto(2) on unconnected socket has the following side effects: 1) After first sendto(2) the "unconnected" socket will receive datagrams destined to the selected port. 2) All subsequent sendto(2) calls will use the same source port. Effectively, such sendto(2) acts like a bind(2) to INADDR_ANY:0. Indeed, if you do this: s1 = socket(PF_INET, SOCK_DGRAM, 0); s2 = socket(PF_INET, SOCK_DGRAM, 0); sendto(s1, ..., &somedestination, ...); bind(s2, &{ .sin_addr = INADDR_ANY, sin_port = 0 }); And then look into kgdb at resulting inpcbs, you would find them equal in all means modulo bound to different anonymous ports. What is even more interesting is that Linux kernel had picked up same behavior, including that "unconnected" socket will receive datagrams. So it seems that such behavior is now an undocumented standard, thus I covered it in recently added tests/sys/netinet/udp_bindings. Now, with the above knowledge at hand, why are we using in_pcbconnect_setup() and in_pcbinshash(), which are supposed to be private to in_pcb.c, to achieve the binding? Let's use public KPI in_pcbbind() on the first sendto(2) and use in_pcbladdr() on all sendto(2)s. Apart from finally hiding these two should be private functions, we no longer acquire global INP_HASH_WLOCK() for every sendto(2) on unconnected socket as well as remove a couple workarounds. [1] https://mail-archive.FreeBSD.org/cgi/mid.cgi?200210141935.aa83883 Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D49043 --- sys/netinet/udp_usrreq.c | 88 ++++++++++++++++-------------------------------- 1 file changed, 29 insertions(+), 59 deletions(-) diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c index 6c855e7a756b..131242ce9859 100644 --- a/sys/netinet/udp_usrreq.c +++ b/sys/netinet/udp_usrreq.c @@ -1135,10 +1135,9 @@ 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. + * udp_send() may need to bind 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. * * We will need network epoch in either case, to safely lookup into * pcb hash. @@ -1292,66 +1291,37 @@ udp_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *addr, error = prison_remote_ip4(td->td_ucred, &sin->sin_addr); if (error) goto release; - /* - * If a local address or port hasn't yet been selected, or if - * the destination address needs to be rewritten due to using - * a special INADDR_ constant, invoke in_pcbconnect_setup() - * to do the heavy lifting. Once a port is selected, we - * commit the binding back to the socket; we also commit the - * binding of the address if in jail. - * - * If we already have a valid binding and we're not - * requesting a destination address rewrite, use a fast path. + * sendto(2) on unconnected UDP socket results in implicit + * binding to INADDR_ANY and anonymous port. This has two + * side effects: + * 1) after first sendto(2) the socket will receive datagrams + * destined to the selected port. + * 2) subsequent sendto(2) calls will use the same source port. */ - if (inp->inp_laddr.s_addr == INADDR_ANY || - inp->inp_lport == 0 || - sin->sin_addr.s_addr == INADDR_ANY || - sin->sin_addr.s_addr == INADDR_BROADCAST) { - if ((flags & PRUS_IPV6) != 0) { - vflagsav = inp->inp_vflag; - inp->inp_vflag |= INP_IPV4; - inp->inp_vflag &= ~INP_IPV6; - } + if (inp->inp_lport == 0) { + struct sockaddr_in wild = { + .sin_family = AF_INET, + .sin_len = sizeof(struct sockaddr_in), + }; + INP_HASH_WLOCK(pcbinfo); - error = in_pcbconnect_setup(inp, sin, &laddr.s_addr, - &lport, &faddr.s_addr, &fport, td->td_ucred); - if ((flags & PRUS_IPV6) != 0) - inp->inp_vflag = vflagsav; - if (error) { - INP_HASH_WUNLOCK(pcbinfo); + error = in_pcbbind(inp, &wild, V_udp_bind_all_fibs ? + 0 : INPBIND_FIB, td->td_ucred); + INP_HASH_WUNLOCK(pcbinfo); + if (error) + goto release; + lport = inp->inp_lport; + laddr = inp->inp_laddr; + } + if (laddr.s_addr == INADDR_ANY) { + error = in_pcbladdr(inp, &sin->sin_addr, &laddr, + td->td_ucred); + if (error) goto release; - } - - /* - * XXXRW: Why not commit the port if the address is - * !INADDR_ANY? - */ - /* Commit the local port if newly assigned. */ - if (inp->inp_laddr.s_addr == INADDR_ANY && - inp->inp_lport == 0) { - INP_WLOCK_ASSERT(inp); - /* - * Remember addr if jailed, to prevent - * rebinding. - */ - if (prison_flag(td->td_ucred, PR_IP4)) - inp->inp_laddr = laddr; - inp->inp_lport = lport; - error = in_pcbinshash(inp); - INP_HASH_WUNLOCK(pcbinfo); - if (error != 0) { - inp->inp_lport = 0; - error = EAGAIN; - goto release; - } - inp->inp_flags |= INP_ANONPORT; - } else - INP_HASH_WUNLOCK(pcbinfo); - } else { - faddr = sin->sin_addr; - fport = sin->sin_port; } + faddr = sin->sin_addr; + fport = sin->sin_port; } else { INP_LOCK_ASSERT(inp); faddr = inp->inp_faddr;