From: bluhm Date: Mon, 8 Aug 2022 12:06:30 +0000 (+0000) Subject: To make protocol input functions MP safe, internet PCB need protection. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=9e8a1cdfd23caf414ca874057442fd8062181c6e;p=openbsd To make protocol input functions MP safe, internet PCB need protection. 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. OK sashan@ --- diff --git a/sys/net/pf.c b/sys/net/pf.c index 7183db91254..f23968f07c7 100644 --- a/sys/net/pf.c +++ b/sys/net/pf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pf.c,v 1.1136 2022/07/20 09:33:11 mbuhl Exp $ */ +/* $OpenBSD: pf.c,v 1.1137 2022/08/08 12:06:30 bluhm Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -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); } diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c index f55eaf3e3fe..e5700bb1849 100644 --- a/sys/netinet/in_pcb.c +++ b/sys/netinet/in_pcb.c @@ -1,4 +1,4 @@ -/* $OpenBSD: in_pcb.c,v 1.269 2022/08/06 15:57:59 bluhm Exp $ */ +/* $OpenBSD: in_pcb.c,v 1.270 2022/08/08 12:06:30 bluhm Exp $ */ /* $NetBSD: in_pcb.c,v 1.25 1996/02/13 23:41:53 christos Exp $ */ /* @@ -403,17 +403,24 @@ in_pcbaddrisavail(struct inpcb *inp, struct sockaddr_in *sin, int wild, } 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 *laddr, int wild, struct inpcb *inp, { 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 *laddr, int wild, struct inpcb *inp, 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 mbuf *nam) { 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 mbuf *nam) 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 mbuf *nam) 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 *table, void *laddrp, u_int lport_arg, break; } } + in_pcbref(match); mtx_leave(&table->inpt_mtx); return (match); @@ -1119,6 +1141,7 @@ in_pcbhashlookup(struct inpcbtable *table, struct in_addr faddr, 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 *table, struct in_addr laddr, 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) { diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c index c4f22ccf784..b5c9be1f990 100644 --- a/sys/netinet/tcp_input.c +++ b/sys/netinet/tcp_input.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tcp_input.c,v 1.375 2022/01/04 06:32:39 yasuoka Exp $ */ +/* $OpenBSD: tcp_input.c,v 1.376 2022/08/08 12:06:30 bluhm Exp $ */ /* $NetBSD: tcp_input.c,v 1.23 1996/02/13 23:43:44 christos Exp $ */ /* @@ -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; } diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c index bd0671b319a..b7d5eb22dec 100644 --- a/sys/netinet/tcp_subr.c +++ b/sys/netinet/tcp_subr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tcp_subr.c,v 1.184 2022/03/02 12:53:15 bluhm Exp $ */ +/* $OpenBSD: tcp_subr.c,v 1.185 2022/08/08 12:06:30 bluhm Exp $ */ /* $NetBSD: tcp_subr.c,v 1.22 1996/02/13 23:44:00 christos Exp $ */ /* @@ -671,7 +671,9 @@ tcp6_ctlinput(int cmd, struct sockaddr *sa, u_int rdomain, void *d) * 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 *sa, u_int rdomain, void *d) 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 *sa, u_int rdomain, void *v) * 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 *sa, u_int rdomain, void *v) * 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 *sa, u_int rdomain, void *v) 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); } diff --git a/sys/netinet/tcp_usrreq.c b/sys/netinet/tcp_usrreq.c index 67035bd4553..6185187ca0b 100644 --- a/sys/netinet/tcp_usrreq.c +++ b/sys/netinet/tcp_usrreq.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tcp_usrreq.c,v 1.183 2022/02/25 23:51:03 guenther Exp $ */ +/* $OpenBSD: tcp_usrreq.c,v 1.184 2022/08/08 12:06:30 bluhm Exp $ */ /* $NetBSD: tcp_usrreq.c,v 1.20 1996/02/13 23:44:16 christos Exp $ */ /* @@ -822,6 +822,7 @@ tcp_ident(void *oldp, size_t *oldlenp, void *newp, size_t newlen, int dodrop) tp = tcp_drop(tp, ECONNABORTED); else error = ESRCH; + in_pcbunref(inp); return (error); } @@ -851,6 +852,7 @@ tcp_ident(void *oldp, size_t *oldlenp, void *newp, size_t newlen, int dodrop) *oldlenp = sizeof (tir); error = copyout((void *)&tir, oldp, sizeof (tir)); + in_pcbunref(inp); return (error); } diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c index 480cbcc7818..bf10be68923 100644 --- a/sys/netinet/udp_usrreq.c +++ b/sys/netinet/udp_usrreq.c @@ -1,4 +1,4 @@ -/* $OpenBSD: udp_usrreq.c,v 1.280 2022/08/06 15:57:59 bluhm Exp $ */ +/* $OpenBSD: udp_usrreq.c,v 1.281 2022/08/08 12:06:30 bluhm Exp $ */ /* $NetBSD: udp_usrreq.c,v 1.28 1996/03/16 23:54:03 christos Exp $ */ /* @@ -570,17 +570,20 @@ udp_input(struct mbuf **mp, int *offp, int proto, int af) 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 *sa, u_int rdomain, void *d) 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 *sa, u_int rdomain, void *d) } 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 *sa, u_int rdomain, void *d) * 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 *sa, u_int rdomain, void *d) * 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 *sa, u_int rdomain, void *v) rdomain); if (inp && inp->inp_socket != NULL) notify(inp, errno); + in_pcbunref(inp); } else in_pcbnotifyall(&udbtable, sa, rdomain, errno, notify); } diff --git a/sys/netinet6/in6_pcb.c b/sys/netinet6/in6_pcb.c index bbd6f88afdc..43a6e739e28 100644 --- a/sys/netinet6/in6_pcb.c +++ b/sys/netinet6/in6_pcb.c @@ -1,4 +1,4 @@ -/* $OpenBSD: in6_pcb.c,v 1.118 2022/08/06 15:57:59 bluhm Exp $ */ +/* $OpenBSD: in6_pcb.c,v 1.119 2022/08/08 12:06:31 bluhm Exp $ */ /* * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project. @@ -208,18 +208,25 @@ in6_pcbaddrisavail(struct inpcb *inp, struct sockaddr_in6 *sin6, int wild, } 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 mbuf *nam) { 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 mbuf *nam) 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 mbuf *nam) 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 *table, const struct in6_addr *faddr, 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 *table, struct in6_addr *laddr, 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) { diff --git a/sys/netinet6/raw_ip6.c b/sys/netinet6/raw_ip6.c index 04a4b45f92b..33bc726bc3f 100644 --- a/sys/netinet6/raw_ip6.c +++ b/sys/netinet6/raw_ip6.c @@ -1,4 +1,4 @@ -/* $OpenBSD: raw_ip6.c,v 1.148 2022/08/06 15:57:59 bluhm Exp $ */ +/* $OpenBSD: raw_ip6.c,v 1.149 2022/08/08 12:06:31 bluhm Exp $ */ /* $KAME: raw_ip6.c,v 1.69 2001/03/04 15:55:44 itojun Exp $ */ /* @@ -318,7 +318,7 @@ rip6_ctlinput(int cmd, struct sockaddr *sa, u_int rdomain, void *d) 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 *sa, u_int rdomain, void *d) * - ignore the MTU change notification. */ icmp6_mtudisc_update((struct ip6ctlparam *)d, valid); + in_pcbunref(in6p); /* * regardless of if we called icmp6_mtudisc_update(),