From nobody Tue Jan 9 00:30:05 2024 X-Original-To: dev-commits-src-branches@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 4T8Bf556GSz55M6W; Tue, 9 Jan 2024 00:30:05 +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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4T8Bf51tMKz4BNZ; Tue, 9 Jan 2024 00:30:05 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1704760205; 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=q7Se/8tYWqcsiT2kunfeUPSnVEqIzYeHb+APY/W+Tmk=; b=H7Om2GXqn1UP5rCCF7XeLqtA0SxfV6+ur/2hpKfJjGrXakD64vTBkK1J+AM1PVsq6mV60v 2ZT0MR/sOHEgzudEKDM9jfQb2L7FiS3p3vLYdUrPt0NalpOPXR/ecR0f+BN93cox03nD4A xecEomk/isUTyyrCMHNXz63Lid8tFI6n3kF6RbwaUpTHDjSuULX7ia2le6QIO4wJZNEymK 9CRLcRXHxZRpAFzDm3YyMTAfLC0fclZDVyMxh5DdjYA1wp8s51BEvg3k3bd/r21F4B/qZp RMVdoQL/x7dhJxW957oSOfyH/jjNQDlL7528mXcuT9bAXCOcp2k4C2XhfLJpWg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1704760205; 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=q7Se/8tYWqcsiT2kunfeUPSnVEqIzYeHb+APY/W+Tmk=; b=gT0xBaWUpUvmLdMMCFUZpFWQD/lIDQNtnvAu4YIrYPCphDPCQ8fxQ/2/x7JZRnuYUeMqXX GMtLS6UTaLB5das0dkTFXMZ2NXkKZbRA5Ie0ZaYjyACpy1l1Vfwa+qV8kW1RjhRmosba/J HzC2MfzWJL07AbBGdTfLNwOXPcjbYPftSIDUMrlq8aZOyubhjsEn3aXx8RIeJJEkLvwkjZ puGg9cYu+SxbXsOtw6jCrK78Li+u5Yh5F/orsosrasRAS2a1H6VIaEhPHqBoSQ37d6oBdE PPk2OwtL4ha7Oj3Nuw5CcEHbgWR9rD78hdGrs0pmVeDOx0xqbQczmYtFp1bwEw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1704760205; a=rsa-sha256; cv=none; b=Xdp9KBE4ofC9dbfBFLugsvIc2ts2zai9b5oMR/DxW9IZMiST2PYOm5059rQrSobgIpIPXo gYWQVD9vDQmvtCYYw+GdwH3lmYtHEgQdn5fPXOFUk9CSQqkeQ6xALaM/+8RbRFDGOY/eC6 umwl7zOeM6edjVAx1Qn/FfFNAN+LhVY9ANXJcjspNuHfhwcIuuCx6O1UdPiMXy0zNJJpmC v2PNMH1FNeZiQUB3P8/94rvts0KHUZeBIAC4OJh0kLhBM4t70coqx+Z8DurYHP3jtKzGdY L4ASh+Q7j1+YcX+evOI3QYZx+I+1e4UdfSt7bEi4LHOS3L/txa9WrzT+s8LdEw== 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 4T8Bf50ygBzSsv; Tue, 9 Jan 2024 00:30:05 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 4090U5rW014561; Tue, 9 Jan 2024 00:30:05 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 4090U5Q6014554; Tue, 9 Jan 2024 00:30:05 GMT (envelope-from git) Date: Tue, 9 Jan 2024 00:30:05 GMT Message-Id: <202401090030.4090U5Q6014554@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Gleb Smirnoff Subject: git: 2bfe735277b8 - stable/14 - inpcb: reoder inpcb destruction List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@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/stable/14 X-Git-Reftype: branch X-Git-Commit: 2bfe735277b8858dd7ad937e0bf2286bdfb45182 Auto-Submitted: auto-generated The branch stable/14 has been updated by glebius: URL: https://cgit.FreeBSD.org/src/commit/?id=2bfe735277b8858dd7ad937e0bf2286bdfb45182 commit 2bfe735277b8858dd7ad937e0bf2286bdfb45182 Author: Gleb Smirnoff AuthorDate: 2023-12-27 16:34:37 +0000 Commit: Gleb Smirnoff CommitDate: 2024-01-09 00:29:38 +0000 inpcb: reoder inpcb destruction First, merge in_pcbdetach() with in_pcbfree(). The comment for in_pcbdetach() was no longer correct. Then, make sure we remove the inpcb from the hash before we commit any destructive actions on it. There are couple functions that rely on the hash lock skipping SMR + inpcb lock to lookup an inpcb. Although there are no known functions that similarly rely on the global inpcb list lock, also do list removal before destructive actions. PR: 273890 Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D43122 (cherry picked from commit a13039e2709277b1c3b159e694cc909a5e044151) --- sys/netinet/in_pcb.c | 39 +++++++++++++++------------------------ sys/netinet/in_pcb.h | 1 - sys/netinet/raw_ip.c | 1 - sys/netinet/tcp_syncache.c | 2 -- sys/netinet/tcp_usrreq.c | 2 -- sys/netinet/udp_usrreq.c | 1 - sys/netinet6/raw_ip6.c | 1 - sys/netinet6/udp6_usrreq.c | 1 - 8 files changed, 15 insertions(+), 33 deletions(-) diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c index 2586c107ceaf..a54b93812c55 100644 --- a/sys/netinet/in_pcb.c +++ b/sys/netinet/in_pcb.c @@ -1405,26 +1405,6 @@ in_pcbdisconnect(struct inpcb *inp) } #endif /* INET */ -/* - * in_pcbdetach() is responsibe for disassociating a socket from an inpcb. - * For most protocols, this will be invoked immediately prior to calling - * in_pcbfree(). However, with TCP the inpcb may significantly outlive the - * socket, in which case in_pcbfree() is deferred. - */ -void -in_pcbdetach(struct inpcb *inp) -{ - - KASSERT(inp->inp_socket != NULL, ("%s: inp_socket == NULL", __func__)); - -#ifdef RATELIMIT - if (inp->inp_snd_tag != NULL) - in_pcbdetach_txrtlmt(inp); -#endif - inp->inp_socket->so_pcb = NULL; - inp->inp_socket = NULL; -} - /* * inpcb hash lookups are protected by SMR section. * @@ -1735,19 +1715,30 @@ in_pcbfree(struct inpcb *inp) #endif INP_WLOCK_ASSERT(inp); - KASSERT(inp->inp_socket == NULL, ("%s: inp_socket != NULL", __func__)); + KASSERT(inp->inp_socket != NULL, ("%s: inp_socket == NULL", __func__)); KASSERT((inp->inp_flags & INP_FREED) == 0, ("%s: called twice for pcb %p", __func__, inp)); - inp->inp_flags |= INP_FREED; + /* + * in_pcblookup_local() and in6_pcblookup_local() may return an inpcb + * from the hash without acquiring inpcb lock, they rely on the hash + * lock, thus in_pcbremhash() should be the first action. + */ + if (inp->inp_flags & INP_INHASHLIST) + in_pcbremhash(inp); INP_INFO_WLOCK(pcbinfo); inp->inp_gencnt = ++pcbinfo->ipi_gencnt; pcbinfo->ipi_count--; CK_LIST_REMOVE(inp, inp_list); INP_INFO_WUNLOCK(pcbinfo); - if (inp->inp_flags & INP_INHASHLIST) - in_pcbremhash(inp); +#ifdef RATELIMIT + if (inp->inp_snd_tag != NULL) + in_pcbdetach_txrtlmt(inp); +#endif + inp->inp_flags |= INP_FREED; + inp->inp_socket->so_pcb = NULL; + inp->inp_socket = NULL; RO_INVALIDATE_CACHE(&inp->inp_route); #ifdef MAC diff --git a/sys/netinet/in_pcb.h b/sys/netinet/in_pcb.h index 19d281937b52..4844bbee3b54 100644 --- a/sys/netinet/in_pcb.h +++ b/sys/netinet/in_pcb.h @@ -672,7 +672,6 @@ int in_pcbconnect(struct inpcb *, struct sockaddr_in *, struct ucred *, bool); int in_pcbconnect_setup(struct inpcb *, struct sockaddr_in *, in_addr_t *, u_short *, in_addr_t *, u_short *, struct ucred *); -void in_pcbdetach(struct inpcb *); void in_pcbdisconnect(struct inpcb *); void in_pcbdrop(struct inpcb *); void in_pcbfree(struct inpcb *); diff --git a/sys/netinet/raw_ip.c b/sys/netinet/raw_ip.c index e6e8b7a56680..04b12b6587dd 100644 --- a/sys/netinet/raw_ip.c +++ b/sys/netinet/raw_ip.c @@ -862,7 +862,6 @@ rip_detach(struct socket *so) ip_rsvp_force_done(so); if (so == V_ip_rsvpd) ip_rsvp_done(); - in_pcbdetach(inp); in_pcbfree(inp); } diff --git a/sys/netinet/tcp_syncache.c b/sys/netinet/tcp_syncache.c index 2c381ef600d6..20c77930556e 100644 --- a/sys/netinet/tcp_syncache.c +++ b/sys/netinet/tcp_syncache.c @@ -803,7 +803,6 @@ syncache_socket(struct syncache *sc, struct socket *lso, struct mbuf *m) } inp = sotoinpcb(so); if ((tp = tcp_newtcpcb(inp)) == NULL) { - in_pcbdetach(inp); in_pcbfree(inp); sodealloc(so); goto allocfail; @@ -1051,7 +1050,6 @@ allocfail: return (NULL); abort: - in_pcbdetach(inp); in_pcbfree(inp); sodealloc(so); if ((s = tcp_log_addrs(&sc->sc_inc, NULL, NULL, NULL))) { diff --git a/sys/netinet/tcp_usrreq.c b/sys/netinet/tcp_usrreq.c index 8b0b3c296c62..767045480abf 100644 --- a/sys/netinet/tcp_usrreq.c +++ b/sys/netinet/tcp_usrreq.c @@ -177,7 +177,6 @@ tcp_usr_attach(struct socket *so, int proto, struct thread *td) tp = tcp_newtcpcb(inp); if (tp == NULL) { error = ENOBUFS; - in_pcbdetach(inp); in_pcbfree(inp); goto out; } @@ -215,7 +214,6 @@ tcp_usr_detach(struct socket *so) ("%s: inp %p not dropped or embryonic", __func__, inp)); tcp_discardcb(tp); - in_pcbdetach(inp); in_pcbfree(inp); } diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c index cbda7f536262..708a4e6b730d 100644 --- a/sys/netinet/udp_usrreq.c +++ b/sys/netinet/udp_usrreq.c @@ -1634,7 +1634,6 @@ udp_detach(struct socket *so) KASSERT(inp->inp_faddr.s_addr == INADDR_ANY, ("udp_detach: not disconnected")); INP_WLOCK(inp); - in_pcbdetach(inp); in_pcbfree(inp); } diff --git a/sys/netinet6/raw_ip6.c b/sys/netinet6/raw_ip6.c index d790a397f551..66fe0afbe918 100644 --- a/sys/netinet6/raw_ip6.c +++ b/sys/netinet6/raw_ip6.c @@ -689,7 +689,6 @@ rip6_detach(struct socket *so) /* xxx: RSVP */ INP_WLOCK(inp); free(inp->in6p_icmp6filt, M_PCB); - in_pcbdetach(inp); in_pcbfree(inp); } diff --git a/sys/netinet6/udp6_usrreq.c b/sys/netinet6/udp6_usrreq.c index 4e69608b71de..35d68e164145 100644 --- a/sys/netinet6/udp6_usrreq.c +++ b/sys/netinet6/udp6_usrreq.c @@ -1203,7 +1203,6 @@ udp6_detach(struct socket *so) KASSERT(inp != NULL, ("udp6_detach: inp == NULL")); INP_WLOCK(inp); - in_pcbdetach(inp); in_pcbfree(inp); }