git: a13039e27092 - main - inpcb: reoder inpcb destruction
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 27 Dec 2023 16:35:42 UTC
The branch main has been updated by glebius:
URL: https://cgit.FreeBSD.org/src/commit/?id=a13039e2709277b1c3b159e694cc909a5e044151
commit a13039e2709277b1c3b159e694cc909a5e044151
Author: Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2023-12-27 16:34:37 +0000
Commit: Gleb Smirnoff <glebius@FreeBSD.org>
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);
}