From nobody Wed Dec 27 16:35:42 2023 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 4T0chH0z5sz55vsp; Wed, 27 Dec 2023 16:35:43 +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 4T0chG51wzz3SKY; Wed, 27 Dec 2023 16:35:42 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1703694942; 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=LTvifUZR6tEnEpzy1JG/yPLeqaGvOduw2CegyD4Ux1Y=; b=Q7xJRO3FE5sbT2bFyyxdMdYKMWNubmuR5u1Qo1+reE6C0qGBtUAO3gwYQdjU7nah1zdBs1 7bC7hWFoYBCx4cjzE5kNiZjJC43ic6Nu1fxDZ5CZ3umyuE967hUMEwevk9enEa/azF1bon f426tu1YOBw/F/VV8+jgSWZVHR2uFCDVRNVRmY/Dxhxs8XBTuU4vBiuRQoEyGS/FMIvi4l Aejfi0HGJmnGfmONr6DjpIqO0aREChUfPx5kmNgDv0CueID415BuIKw5pTeR5PVZ4PSF9l LS0Y1UUHCTndv2K2udvm+dsJNyvtTEiYh0BccrH9v8uwiXqDTaKHPeryRtJSEQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1703694942; a=rsa-sha256; cv=none; b=lq/PWQMfUGrp1+CVrqIWOZUoApjASbDlU/OAbSN3mqe3GVqb3r+56ttrAEY4hiHimP6Uae ET9wMC0nBod8nUrJWGP84sLAlrmQTP3YC0jD82hRwWOr+elK+YfPKcKqn5eBKzxi08JYKj H8vEGXjhHo22kee6WVQ4yrlSSrcgojR0WVVaGRMjz8Hu9MSaolAboLz+KKugWvu3Uz9A39 qIx5uojHljXSKB+RvDrpppWOmgY0Xvy6xJjlgfjh6mv5GSfRBQKydDWgG7b1X4XiW1o5NA fCM0kaZQ93aZmzvcHQ657GVqNhR5q6Sc43u9yVjrt4VZf0ibAdGfBm8MEq5tWQ== 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=1703694942; 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=LTvifUZR6tEnEpzy1JG/yPLeqaGvOduw2CegyD4Ux1Y=; b=lbk289uSQ9NQZCO6NF5JHsXTQHeXth6PC6QFuGZlCvg4EMPvwD5D2+Kyb/ETOcvpK5j7QU /MRj1L/NVZgwHrqoeykyQl/MJimmgUfSc9n37QFVFysJVQoPHqlj6vMUqyk1Bl6Mnk2kfD 1Ls/EC3I9E6ToA6gkhHa2WO95waN6HBhrFFEv4A2K1ydfQYIEsa+C5w6j2PI5aJfQPdhwt +wCRr7r2GO1p/e8hPwcXBzzhzy3Iet5nLjJoRAcQRTPw3sDakJb5lVSNtIyaLKjm7UI1SC 8o4t29OsHB7HQQZM8vN6QELSHmyh6HxWDvHdQUyPgJK7tIri6eNeqD58rMo/8Q== 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 4T0chG450nzmRD; Wed, 27 Dec 2023 16:35:42 +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 3BRGZgdt081746; Wed, 27 Dec 2023 16:35:42 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 3BRGZgLd081743; Wed, 27 Dec 2023 16:35:42 GMT (envelope-from git) Date: Wed, 27 Dec 2023 16:35:42 GMT Message-Id: <202312271635.3BRGZgLd081743@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: a13039e27092 - main - inpcb: reoder inpcb destruction 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: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: 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: a13039e2709277b1c3b159e694cc909a5e044151 Auto-Submitted: auto-generated The branch main has been updated by glebius: URL: https://cgit.FreeBSD.org/src/commit/?id=a13039e2709277b1c3b159e694cc909a5e044151 commit a13039e2709277b1c3b159e694cc909a5e044151 Author: Gleb Smirnoff AuthorDate: 2023-12-27 16:34:37 +0000 Commit: Gleb Smirnoff CommitDate: 2023-12-27 16:34:37 +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 --- 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 0d763184f68c..63b4fc57230e 100644 --- a/sys/netinet/in_pcb.c +++ b/sys/netinet/in_pcb.c @@ -1403,26 +1403,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. * @@ -1733,19 +1713,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 adde89dde873..3d633741dc27 100644 --- a/sys/netinet/in_pcb.h +++ b/sys/netinet/in_pcb.h @@ -670,7 +670,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 2a1e3dfb8616..4a61e685d898 100644 --- a/sys/netinet/raw_ip.c +++ b/sys/netinet/raw_ip.c @@ -860,7 +860,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 d3ba42fd9d06..dad79374c08b 100644 --- a/sys/netinet/tcp_usrreq.c +++ b/sys/netinet/tcp_usrreq.c @@ -175,7 +175,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; } @@ -213,7 +212,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 77cbf9d98c83..f7e0f7417818 100644 --- a/sys/netinet/udp_usrreq.c +++ b/sys/netinet/udp_usrreq.c @@ -1641,7 +1641,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 266925836329..174cc29e6008 100644 --- a/sys/netinet6/raw_ip6.c +++ b/sys/netinet6/raw_ip6.c @@ -687,7 +687,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 681645880368..bbd7ba9477d6 100644 --- a/sys/netinet6/udp6_usrreq.c +++ b/sys/netinet6/udp6_usrreq.c @@ -1201,7 +1201,6 @@ udp6_detach(struct socket *so) KASSERT(inp != NULL, ("udp6_detach: inp == NULL")); INP_WLOCK(inp); - in_pcbdetach(inp); in_pcbfree(inp); }