Hi,
To make protocol input functions MP safe, internet PCB need protection.
Let's use their reference counter in more places.
The in_pcb lookup functions hold the PCBs in hash tables protected
by table->inpt_mtx mutex. Whenever a result is returned, increment
the ref count before releasing the mutex. Then the inp can be used
as long as neccessary. Unref it at the end of all functions that
call in_pcb lookup.
As a shortcut, pf may also hold a reference to the PCB. When
pf_inp_lookup() returns it, it also incements the ref count and the
caller can handle it like the inp from table lookup.
Currently changing the hash tables is protected by exclusive netlock.
If we want to get rid of it, ref count seems the way to go. Also
we have some hacks with tcpcb lifetime in TCP timeouts. Refcount
may also solve that later.
This diff only a small step covering the references from PCB tables.
When testing this diff, have a look at "vmstat -m" inpcb "In Use"
counter.
Name Size Requests Fail InUse Pgreq Pgrel Npage Hiwat Minpg Maxpg Idle
inpcb 220 515861 0 17 346 344 2 96 0 8 0
It should be equal to number of sockets in "netstat -an -f inet"
plus "netstat -an -f inet6". If in use grows permanently, I have
introduced a leak. Regress tests did not find one.
ok?
bluhm
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1136
diff -u -p -r1.1136 pf.c
--- net/pf.c 20 Jul 2022 09:33:11 -0000 1.1136
+++ net/pf.c 4 Aug 2022 22:29:09 -0000
@@ -3375,6 +3375,7 @@ pf_socket_lookup(struct pf_pdesc *pd)
pd->lookup.uid = inp->inp_socket->so_euid;
pd->lookup.gid = inp->inp_socket->so_egid;
pd->lookup.pid = inp->inp_socket->so_cpid;
+ in_pcbunref(inp);
return (1);
}
@@ -7531,6 +7532,7 @@ pf_inp_lookup(struct mbuf *m)
if (inp && inp->inp_pf_sk)
KASSERT(m->m_pkthdr.pf.statekey == inp->inp_pf_sk);
+ in_pcbref(inp);
return (inp);
}
Index: netinet/in_pcb.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.268
diff -u -p -r1.268 in_pcb.c
--- netinet/in_pcb.c 28 Jun 2022 09:32:27 -0000 1.268
+++ netinet/in_pcb.c 4 Aug 2022 22:29:09 -0000
@@ -403,17 +403,24 @@ in_pcbaddrisavail(struct inpcb *inp, str
}
if (lport) {
struct inpcb *t;
+ int error = 0;
if (so->so_euid && !IN_MULTICAST(sin->sin_addr.s_addr)) {
t = in_pcblookup_local(table, &sin->sin_addr, lport,
INPLOOKUP_WILDCARD, inp->inp_rtableid);
if (t && (so->so_euid != t->inp_socket->so_euid))
- return (EADDRINUSE);
+ error = EADDRINUSE;
+ in_pcbunref(t);
+ if (error)
+ return (error);
}
t = in_pcblookup_local(table, &sin->sin_addr, lport,
wild, inp->inp_rtableid);
if (t && (reuseport & t->inp_socket->so_options) == 0)
- return (EADDRINUSE);
+ error = EADDRINUSE;
+ in_pcbunref(t);
+ if (error)
+ return (error);
}
return (0);
@@ -425,6 +432,7 @@ in_pcbpickport(u_int16_t *lport, void *l
{
struct socket *so = inp->inp_socket;
struct inpcbtable *table = inp->inp_table;
+ struct inpcb *t;
u_int16_t first, last, lower, higher, candidate, localport;
int count;
@@ -456,16 +464,20 @@ in_pcbpickport(u_int16_t *lport, void *l
count = higher - lower;
candidate = lower + arc4random_uniform(count);
+ t = NULL;
do {
- if (count-- < 0) /* completely used? */
- return (EADDRNOTAVAIL);
- ++candidate;
- if (candidate < lower || candidate > higher)
- candidate = lower;
- localport = htons(candidate);
- } while (in_baddynamic(candidate, so->so_proto->pr_protocol) ||
- in_pcblookup_local(table, laddr, localport, wild,
- inp->inp_rtableid));
+ in_pcbunref(t);
+ do {
+ if (count-- < 0) /* completely used? */
+ return (EADDRNOTAVAIL);
+ ++candidate;
+ if (candidate < lower || candidate > higher)
+ candidate = lower;
+ localport = htons(candidate);
+ } while (in_baddynamic(candidate, so->so_proto->pr_protocol));
+ t = in_pcblookup_local(table, laddr, localport, wild,
+ inp->inp_rtableid);
+ } while (t != NULL);
*lport = localport;
return (0);
@@ -482,6 +494,7 @@ in_pcbconnect(struct inpcb *inp, struct
{
struct in_addr ina;
struct sockaddr_in *sin;
+ struct inpcb *t;
int error;
#ifdef INET6
@@ -498,9 +511,12 @@ in_pcbconnect(struct inpcb *inp, struct
if (error)
return (error);
- if (in_pcbhashlookup(inp->inp_table, sin->sin_addr, sin->sin_port,
- ina, inp->inp_lport, inp->inp_rtableid) != NULL)
+ t = in_pcbhashlookup(inp->inp_table, sin->sin_addr, sin->sin_port,
+ ina, inp->inp_lport, inp->inp_rtableid);
+ if (t != NULL) {
+ in_pcbunref(t);
return (EADDRINUSE);
+ }
KASSERT(inp->inp_laddr.s_addr == INADDR_ANY || inp->inp_lport);
@@ -509,10 +525,12 @@ in_pcbconnect(struct inpcb *inp, struct
error = in_pcbbind(inp, NULL, curproc);
if (error)
return (error);
- if (in_pcbhashlookup(inp->inp_table, sin->sin_addr,
+ t = in_pcbhashlookup(inp->inp_table, sin->sin_addr,
sin->sin_port, ina, inp->inp_lport,
- inp->inp_rtableid) != NULL) {
+ inp->inp_rtableid);
+ if (t != NULL) {
inp->inp_lport = 0;
+ in_pcbunref(t);
return (EADDRINUSE);
}
}
@@ -603,23 +621,26 @@ in_pcbdetach(struct inpcb *inp)
struct inpcb *
in_pcbref(struct inpcb *inp)
{
- if (inp != NULL)
- refcnt_take(&inp->inp_refcnt);
+ if (inp == NULL)
+ return NULL;
+ refcnt_take(&inp->inp_refcnt);
return inp;
}
void
in_pcbunref(struct inpcb *inp)
{
- if (refcnt_rele(&inp->inp_refcnt)) {
- KASSERT((LIST_NEXT(inp, inp_hash) == NULL) ||
- (LIST_NEXT(inp, inp_hash) == _Q_INVALID));
- KASSERT((LIST_NEXT(inp, inp_lhash) == NULL) ||
- (LIST_NEXT(inp, inp_lhash) == _Q_INVALID));
- KASSERT((TAILQ_NEXT(inp, inp_queue) == NULL) ||
- (TAILQ_NEXT(inp, inp_queue) == _Q_INVALID));
- pool_put(&inpcb_pool, inp);
- }
+ if (inp == NULL)
+ return;
+ if (refcnt_rele(&inp->inp_refcnt) == 0)
+ return;
+ KASSERT((LIST_NEXT(inp, inp_hash) == NULL) ||
+ (LIST_NEXT(inp, inp_hash) == _Q_INVALID));
+ KASSERT((LIST_NEXT(inp, inp_lhash) == NULL) ||
+ (LIST_NEXT(inp, inp_lhash) == _Q_INVALID));
+ KASSERT((TAILQ_NEXT(inp, inp_queue) == NULL) ||
+ (TAILQ_NEXT(inp, inp_queue) == _Q_INVALID));
+ pool_put(&inpcb_pool, inp);
}
void
@@ -830,6 +851,7 @@ in_pcblookup_local(struct inpcbtable *ta
break;
}
}
+ in_pcbref(match);
mtx_leave(&table->inpt_mtx);
return (match);
@@ -1119,6 +1141,7 @@ in_pcbhashlookup(struct inpcbtable *tabl
break;
}
}
+ in_pcbref(inp);
mtx_leave(&table->inpt_mtx);
#ifdef DIAGNOSTIC
if (inp == NULL && in_pcbnotifymiss) {
@@ -1218,6 +1241,7 @@ in_pcblookup_listen(struct inpcbtable *t
LIST_REMOVE(inp, inp_hash);
LIST_INSERT_HEAD(head, inp, inp_hash);
}
+ in_pcbref(inp);
mtx_leave(&table->inpt_mtx);
#ifdef DIAGNOSTIC
if (inp == NULL && in_pcbnotifymiss) {
Index: netinet/tcp_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.375
diff -u -p -r1.375 tcp_input.c
--- netinet/tcp_input.c 4 Jan 2022 06:32:39 -0000 1.375
+++ netinet/tcp_input.c 4 Aug 2022 22:29:09 -0000
@@ -723,7 +723,8 @@ findpcb:
* full-blown connection.
*/
tp = NULL;
- inp = sotoinpcb(so);
+ in_pcbunref(inp);
+ inp = in_pcbref(sotoinpcb(so));
tp = intotcpcb(inp);
if (tp == NULL)
goto badsyn; /*XXX*/
@@ -832,6 +833,7 @@ findpcb:
tcpstat_inc(tcps_dropsyn);
goto drop;
}
+ in_pcbunref(inp);
return IPPROTO_DONE;
}
}
@@ -1002,6 +1004,7 @@ findpcb:
if (so->so_snd.sb_cc ||
tp->t_flags & TF_NEEDOUTPUT)
(void) tcp_output(tp);
+ in_pcbunref(inp);
return IPPROTO_DONE;
}
} else if (th->th_ack == tp->snd_una &&
@@ -1050,6 +1053,7 @@ findpcb:
tp->t_flags &= ~TF_BLOCKOUTPUT;
if (tp->t_flags & (TF_ACKNOW|TF_NEEDOUTPUT))
(void) tcp_output(tp);
+ in_pcbunref(inp);
return IPPROTO_DONE;
}
}
@@ -1244,6 +1248,7 @@ trimthenstep6:
((arc4random() & 0x7fffffff) | 0x8000);
reuse = &iss;
tp = tcp_close(tp);
+ in_pcbunref(inp);
inp = NULL;
goto findpcb;
}
@@ -2028,6 +2033,7 @@ dodata:
/* XXX */
*/
if (tp->t_flags & (TF_ACKNOW|TF_NEEDOUTPUT))
(void) tcp_output(tp);
+ in_pcbunref(inp);
return IPPROTO_DONE;
badsyn:
@@ -2056,6 +2062,7 @@ dropafterack:
m_freem(m);
tp->t_flags |= TF_ACKNOW;
(void) tcp_output(tp);
+ in_pcbunref(inp);
return IPPROTO_DONE;
dropwithreset_ratelim:
@@ -2090,6 +2097,7 @@ dropwithreset:
(tcp_seq)0, TH_RST|TH_ACK, m->m_pkthdr.ph_rtableid);
}
m_freem(m);
+ in_pcbunref(inp);
return IPPROTO_DONE;
drop:
@@ -2100,6 +2108,7 @@ drop:
tcp_trace(TA_DROP, ostate, tp, otp, saveti, 0, tlen);
m_freem(m);
+ in_pcbunref(inp);
return IPPROTO_DONE;
}
Index: netinet/tcp_subr.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_subr.c,v
retrieving revision 1.184
diff -u -p -r1.184 tcp_subr.c
--- netinet/tcp_subr.c 2 Mar 2022 12:53:15 -0000 1.184
+++ netinet/tcp_subr.c 4 Aug 2022 22:29:09 -0000
@@ -671,7 +671,9 @@ tcp6_ctlinput(int cmd, struct sockaddr *
* corresponding routing entry, or
* - ignore the MTU change notification.
*/
- icmp6_mtudisc_update((struct ip6ctlparam *)d, inp !=
NULL);
+ icmp6_mtudisc_update((struct ip6ctlparam *)d,
+ inp != NULL);
+ in_pcbunref(inp);
return;
}
if (inp) {
@@ -686,6 +688,7 @@ tcp6_ctlinput(int cmd, struct sockaddr *
inet6ctlerrmap[cmd] == EHOSTDOWN)
syn_cache_unreach((struct sockaddr *)sa6_src,
sa, &th, rdomain);
+ in_pcbunref(inp);
} else {
in6_pcbnotify(&tcbtable, sa6, 0,
sa6_src, 0, rdomain, cmd, NULL, notify);
@@ -746,8 +749,10 @@ tcp_ctlinput(int cmd, struct sockaddr *s
* ever sent, drop the message.
*/
mtu = (u_int)ntohs(icp->icmp_nextmtu);
- if (mtu >= tp->t_pmtud_mtu_sent)
+ if (mtu >= tp->t_pmtud_mtu_sent) {
+ in_pcbunref(inp);
return;
+ }
if (mtu >= tcp_hdrsz(tp) + tp->t_pmtud_mss_acked) {
/*
* Calculate new MTU, and create corresponding
@@ -764,20 +769,25 @@ tcp_ctlinput(int cmd, struct sockaddr *s
* refers to an older TCP segment
*/
if (tp->t_flags & TF_PMTUD_PEND) {
- if (SEQ_LT(tp->t_pmtud_th_seq, seq))
+ if (SEQ_LT(tp->t_pmtud_th_seq, seq)) {
+ in_pcbunref(inp);
return;
+ }
} else
tp->t_flags |= TF_PMTUD_PEND;
tp->t_pmtud_th_seq = seq;
tp->t_pmtud_nextmtu = icp->icmp_nextmtu;
tp->t_pmtud_ip_len = icp->icmp_ip.ip_len;
tp->t_pmtud_ip_hl = icp->icmp_ip.ip_hl;
+ in_pcbunref(inp);
return;
}
} else {
/* ignore if we don't have a matching connection */
+ in_pcbunref(inp);
return;
}
+ in_pcbunref(inp);
notify = tcp_mtudisc, ip = 0;
} else if (cmd == PRC_MTUINC)
notify = tcp_mtudisc_increase, ip = 0;
@@ -810,6 +820,7 @@ tcp_ctlinput(int cmd, struct sockaddr *s
sin.sin_addr = ip->ip_src;
syn_cache_unreach(sintosa(&sin), sa, th, rdomain);
}
+ in_pcbunref(inp);
} else
in_pcbnotifyall(&tcbtable, sa, rdomain, errno, notify);
}
Index: netinet/tcp_usrreq.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_usrreq.c,v
retrieving revision 1.183
diff -u -p -r1.183 tcp_usrreq.c
--- netinet/tcp_usrreq.c 25 Feb 2022 23:51:03 -0000 1.183
+++ netinet/tcp_usrreq.c 4 Aug 2022 22:34:40 -0000
@@ -822,6 +822,7 @@ tcp_ident(void *oldp, size_t *oldlenp, v
tp = tcp_drop(tp, ECONNABORTED);
else
error = ESRCH;
+ in_pcbunref(inp);
return (error);
}
@@ -851,6 +852,7 @@ tcp_ident(void *oldp, size_t *oldlenp, v
*oldlenp = sizeof (tir);
error = copyout((void *)&tir, oldp, sizeof (tir));
+ in_pcbunref(inp);
return (error);
}
Index: netinet/udp_usrreq.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.279
diff -u -p -r1.279 udp_usrreq.c
--- netinet/udp_usrreq.c 26 Jun 2022 15:50:21 -0000 1.279
+++ netinet/udp_usrreq.c 4 Aug 2022 22:29:09 -0000
@@ -570,17 +570,20 @@ udp_input(struct mbuf **mp, int *offp, i
m = *mp = pipex_l2tp_input(m, off, session,
ipsecflowinfo);
pipex_rele_session(session);
-
- if (m == NULL)
+ if (m == NULL) {
+ in_pcbunref(inp);
return IPPROTO_DONE;
+ }
}
}
#endif
udp_sbappend(inp, m, ip, ip6, iphlen, uh, &srcsa.sa, ipsecflowinfo);
+ in_pcbunref(inp);
return IPPROTO_DONE;
bad:
m_freem(m);
+ in_pcbunref(inp);
return IPPROTO_DONE;
}
@@ -674,6 +677,7 @@ udp6_ctlinput(int cmd, struct sockaddr *
u_int16_t uh_sport;
u_int16_t uh_dport;
} *uhp;
+ struct inpcb *inp;
void (*notify)(struct inpcb *, int) = udp_notify;
if (sa == NULL)
@@ -759,17 +763,14 @@ udp6_ctlinput(int cmd, struct sockaddr *
}
if (cmd == PRC_MSGSIZE) {
- int valid = 0;
-
/*
* Check to see if we have a valid UDP socket
* corresponding to the address in the ICMPv6 message
* payload.
*/
- if (in6_pcbhashlookup(&udbtable, &sa6.sin6_addr,
+ inp = in6_pcbhashlookup(&udbtable, &sa6.sin6_addr,
uh.uh_dport, &sa6_src.sin6_addr, uh.uh_sport,
- rdomain))
- valid = 1;
+ rdomain);
#if 0
/*
* As the use of sendto(2) is fairly popular,
@@ -778,10 +779,11 @@ udp6_ctlinput(int cmd, struct sockaddr *
* We should at least check if the local address (= s)
* is really ours.
*/
- else if (in6_pcblookup_listen(&udbtable,
- &sa6_src.sin6_addr, uh.uh_sport, NULL,
- rdomain))
- valid = 1;
+ if (inp == NULL) {
+ inp = in6_pcblookup_listen(&udbtable,
+ &sa6_src.sin6_addr, uh.uh_sport, NULL,
+ rdomain))
+ }
#endif
/*
@@ -791,7 +793,9 @@ udp6_ctlinput(int cmd, struct sockaddr *
* corresponding routing entry, or
* - ignore the MTU change notification.
*/
- icmp6_mtudisc_update((struct ip6ctlparam *)d, valid);
+ icmp6_mtudisc_update((struct ip6ctlparam *)d,
+ inp != NULL);
+ in_pcbunref(inp);
/*
* regardless of if we called icmp6_mtudisc_update(),
@@ -855,6 +859,7 @@ udp_ctlinput(int cmd, struct sockaddr *s
rdomain);
if (inp && inp->inp_socket != NULL)
notify(inp, errno);
+ in_pcbunref(inp);
} else
in_pcbnotifyall(&udbtable, sa, rdomain, errno, notify);
}
Index: netinet6/in6_pcb.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6_pcb.c,v
retrieving revision 1.117
diff -u -p -r1.117 in6_pcb.c
--- netinet6/in6_pcb.c 14 Apr 2022 14:10:22 -0000 1.117
+++ netinet6/in6_pcb.c 4 Aug 2022 22:29:09 -0000
@@ -208,18 +208,25 @@ in6_pcbaddrisavail(struct inpcb *inp, st
}
if (lport) {
struct inpcb *t;
+ int error = 0;
if (so->so_euid && !IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr)) {
t = in_pcblookup_local(table, &sin6->sin6_addr, lport,
INPLOOKUP_WILDCARD | INPLOOKUP_IPV6,
inp->inp_rtableid);
if (t && (so->so_euid != t->inp_socket->so_euid))
- return (EADDRINUSE);
+ error = EADDRINUSE;
+ in_pcbunref(t);
+ if (error)
+ return (error);
}
t = in_pcblookup_local(table, &sin6->sin6_addr, lport,
wild, inp->inp_rtableid);
if (t && (reuseport & t->inp_socket->so_options) == 0)
- return (EADDRINUSE);
+ error = EADDRINUSE;
+ in_pcbunref(t);
+ if (error)
+ return (error);
}
return (0);
}
@@ -237,6 +244,7 @@ in6_pcbconnect(struct inpcb *inp, struct
{
struct in6_addr *in6a = NULL;
struct sockaddr_in6 *sin6;
+ struct inpcb *t;
int error;
struct sockaddr_in6 tmp;
@@ -272,9 +280,11 @@ in6_pcbconnect(struct inpcb *inp, struct
inp->inp_ipv6.ip6_hlim = (u_int8_t)in6_selecthlim(inp);
- if (in6_pcbhashlookup(inp->inp_table, &sin6->sin6_addr, sin6->sin6_port,
+ t = in6_pcbhashlookup(inp->inp_table, &sin6->sin6_addr, sin6->sin6_port,
IN6_IS_ADDR_UNSPECIFIED(&inp->inp_laddr6) ? in6a : &inp->inp_laddr6,
- inp->inp_lport, inp->inp_rtableid) != NULL) {
+ inp->inp_lport, inp->inp_rtableid);
+ if (t != NULL) {
+ in_pcbunref(t);
return (EADDRINUSE);
}
@@ -285,10 +295,12 @@ in6_pcbconnect(struct inpcb *inp, struct
error = in_pcbbind(inp, NULL, curproc);
if (error)
return (error);
- if (in6_pcbhashlookup(inp->inp_table, &sin6->sin6_addr,
+ t = in6_pcbhashlookup(inp->inp_table, &sin6->sin6_addr,
sin6->sin6_port, in6a, inp->inp_lport,
- inp->inp_rtableid) != NULL) {
+ inp->inp_rtableid);
+ if (t != NULL) {
inp->inp_lport = 0;
+ in_pcbunref(t);
return (EADDRINUSE);
}
}
@@ -535,6 +547,7 @@ in6_pcbhashlookup(struct inpcbtable *tab
break;
}
}
+ in_pcbref(inp);
mtx_leave(&table->inpt_mtx);
#ifdef DIAGNOSTIC
if (inp == NULL && in_pcbnotifymiss) {
@@ -619,6 +632,7 @@ in6_pcblookup_listen(struct inpcbtable *
LIST_REMOVE(inp, inp_hash);
LIST_INSERT_HEAD(head, inp, inp_hash);
}
+ in_pcbref(inp);
mtx_leave(&table->inpt_mtx);
#ifdef DIAGNOSTIC
if (inp == NULL && in_pcbnotifymiss) {
Index: netinet6/raw_ip6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/raw_ip6.c,v
retrieving revision 1.147
diff -u -p -r1.147 raw_ip6.c
--- netinet6/raw_ip6.c 23 Mar 2022 00:16:07 -0000 1.147
+++ netinet6/raw_ip6.c 4 Aug 2022 22:29:09 -0000
@@ -318,7 +318,7 @@ rip6_ctlinput(int cmd, struct sockaddr *
if (in6p && in6p->inp_ipv6.ip6_nxt &&
in6p->inp_ipv6.ip6_nxt == nxt)
- valid++;
+ valid = 1;
/*
* Depending on the value of "valid" and routing table
@@ -328,6 +328,7 @@ rip6_ctlinput(int cmd, struct sockaddr *
* - ignore the MTU change notification.
*/
icmp6_mtudisc_update((struct ip6ctlparam *)d, valid);
+ in_pcbunref(in6p);
/*
* regardless of if we called icmp6_mtudisc_update(),
