From nobody Wed Feb 14 06:05:43 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 4TZSNm0vg7z5B4yf; Wed, 14 Feb 2024 06:05:44 +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 4TZSNl6hvRz4Rng; Wed, 14 Feb 2024 06:05:43 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1707890743; 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=+2xB9JoBFaYAInMqiwGa9s1w7zq+Ra1FDk0YVfdGUUg=; b=gfSIpjBU95n/MEjXUn8H3tanndqxpQEfFsXFYIhyGO6Buj13rKWvNBZyekQ0+tACWcg/fj 0duzqmrh1JvUAu17r+yUnWC1EVJNNE+cfHy7RRQ2qs17DdgDix1AXSY4xmHhMSCShuTp8V sulOibQRJnSMeVGKLlwZCfhBQQEbFkSAHlvkVfS1/zpUR2iYlssHKda4BStr77IzwV9jYa dWOLLp8lAeYw0Tnl68JopnDUVvbXQO2uXAvSXH4/q0PcYFUHLPBxqxNh12YEhK4GG+5cZb mc//wCZzfJt3KnbActIJunqxch4GTcBfSzQFcoqAd5HP1aDjyL73BvjU26v10Q== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1707890743; a=rsa-sha256; cv=none; b=xU6SB7LlKEFbL2wbYPiTBCMVkIoCoeQ9JqLbrLiq7nGctpn7UkIwy8KWSGeeLHdI9q4k/S Ko6WPkfSOhLQpqqjfmLg3UA810UIoRhxatb+w0bI3QJ8XXnCEqRwgA7NZB2FaCHu/nxALV WnKPBxUU7C9/pfKXFyfjMhEHyldlHotZYofhgydvkICVeSRRQ0y6J0Oc2U4yhzjHrznKwr bCFMuzXtVlcefUZJB5oihu3QKtU7ZR121CNxFFfPYTtfncxkLPKm66gRHTADWuBWu6Pu/f DvyiqmBk6WfSdxZ/6tR3uFXK446xsui788iC5IEO+N9wwRUfLmptv/xlE0KnZA== 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=1707890743; 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=+2xB9JoBFaYAInMqiwGa9s1w7zq+Ra1FDk0YVfdGUUg=; b=gt1iqdHz9lLIxd9hwLSx09bbGwCfMRmU4GKsgoA7Pk4CYkSbE2MPvtwqUq9U6jR51W/88j diiYtkQIhuvER4/TPtDYkMc2u+QCrdS3zmTKczmlXDYfUNkkBmbmIOOaj4dhPCKff70Bde xVqZxJ4JYRzQXlpz2sMjIH3K2XGi94P5ZdvwotXsQG+7TnoX1/WdyjvdjG88xthHOOU3G6 7/iQrEHmsIAnECVd7gWYfx7VMQKVgGaMKz3R9x8fn+jIoQ3gN1jfRNjkbUccoFIltBSBL8 lnQ1d8IebZv8S4UMDxkCLU82t16+OU1P+xm7EIs3ooj5mFirRH32fjCyhwyPvg== 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 4TZSNl5pJZzn4Z; Wed, 14 Feb 2024 06:05:43 +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 41E65hCK084626; Wed, 14 Feb 2024 06:05:43 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 41E65hQY084623; Wed, 14 Feb 2024 06:05:43 GMT (envelope-from git) Date: Wed, 14 Feb 2024 06:05:43 GMT Message-Id: <202402140605.41E65hQY084623@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Gordon Tetlow Subject: git: 9db5ae3ec45f - releng/14.0 - 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: gordon X-Git-Repository: src X-Git-Refname: refs/heads/releng/14.0 X-Git-Reftype: branch X-Git-Commit: 9db5ae3ec45f069f48808a2916ea1c5374e75e6c Auto-Submitted: auto-generated The branch releng/14.0 has been updated by gordon: URL: https://cgit.FreeBSD.org/src/commit/?id=9db5ae3ec45f069f48808a2916ea1c5374e75e6c commit 9db5ae3ec45f069f48808a2916ea1c5374e75e6c Author: Gleb Smirnoff AuthorDate: 2023-12-27 16:34:37 +0000 Commit: Gordon Tetlow CommitDate: 2024-02-14 05:40:23 +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 Approved by: so Security: FreeBSD-EN-24:04.ip (cherry picked from commit a13039e2709277b1c3b159e694cc909a5e044151) (cherry picked from commit 2bfe735277b8858dd7ad937e0bf2286bdfb45182) --- 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 ffde6a4b88c9..6faf635213b1 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; @@ -1050,7 +1049,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); }