Date: Tue, 9 Jan 2024 00:30:05 GMT From: Gleb Smirnoff <glebius@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 2bfe735277b8 - stable/14 - inpcb: reoder inpcb destruction Message-ID: <202401090030.4090U5Q6014554@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/14 has been updated by glebius: URL: https://cgit.FreeBSD.org/src/commit/?id=2bfe735277b8858dd7ad937e0bf2286bdfb45182 commit 2bfe735277b8858dd7ad937e0bf2286bdfb45182 Author: Gleb Smirnoff <glebius@FreeBSD.org> AuthorDate: 2023-12-27 16:34:37 +0000 Commit: Gleb Smirnoff <glebius@FreeBSD.org> 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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202401090030.4090U5Q6014554>