On Sun, Nov 14, 2021 at 10:50:34PM +0100, Alexander Bluhm wrote: > On Sat, Nov 13, 2021 at 06:04:07PM +0100, Alexander Bluhm wrote: > > It passes regress but there are setups that are not covered. Bridge > > and pfsync with IPsec and TCP signature need special care. > > > > When testing, please check for tdb leaks. The vmstat -m tdb in use > > culumn must be 0. It it is not, try ipsecctl -F. If ipsecctl -sa > > shows nothing, but vmstat -m shows a used tdb, then it is a bug. > > New diff with fix from mvs@. Please continue testing with this one. >
This inherited use-after-free issue. Do you remember I pointed you we it in (*xf_input)() and (*xf_output) paths? When `tdb' exceeds hard limit it killed by tdb_delete(). But this kill is delayed until the tdb_reaper() timeout(9) handler really kills it. So we have use-after-free but don't catch it. Your diff introduced tdb_unref() into tdb_delete(). So when we have `tdb' with exceeds hard limit we decrement it's reference counter and we still schedule timeout to run but we don't expose to caller what this `tdb' is dead. But now we do tdb_unref() after (*xf_input)() and (*xf_output) on this killed `tdb' and we hit assertion. I used ah_input() path to explain: 860 tdb_delete(struct tdb *tdbp) 861 { 862 NET_ASSERT_LOCKED(); 863 864 tdb_unlink(tdbp); 865 /* release tdb_onext/tdb_inext references */ 866 tdb_unbundle(tdbp); 867 /* release the reference for tdb_unlink() */ 868 tdb_unref(tdbp); 869 } 530 ah_input(struct mbuf **mp, struct tdb *tdb, int skip, int protoff) 531 { ... 590 if (hl * sizeof(u_int32_t) != ahx->authsize + rplen - AH_FLENGTH) { 595 ahstat_inc(ahs_badauthl); We follow error path and we return 'IPPROTO_DONE' but `tdb' passed by caller is alive. 596 goto drop; 597 } ... 614 /* Hard expiration. */ 615 if (tdb->tdb_flags & TDBF_BYTES && 616 tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes) { 617 pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD); 618 tdb_delete(tdb); We follow error path and we return 'IPPROTO_DONE' but `tdb' passed by is _dead_ because we destroyed it by tdb_delete(). Actually we only decremented it's reference number. Caller can't distinguish this error case from the previous where `tdb' is alive. 619 goto drop; 620 } ... 847 drop: 848 free(ptr, M_XDATA, 0); 849 m_freemp(mp); 850 crypto_freereq(crp); 851 return IPPROTO_DONE; 852 } 179 ipsec_common_input(struct mbuf **mp, int skip, int protoff, ..., 180 int udpencap) 181 { ... 346 prot = (*(tdbp->tdb_xform->xf_input))(mp, tdbp, skip, protoff); `tdbp' Could be killed here. Actually it don't but it's reference counter is already decremented. So the use-after-free at line 349 is hidden, but the assertion in tdb_unref() at line 351 fill be fired. In the case when the dead `tdb' has more then one reference the another tdb_unref() will hit assertion. 347 if (prot == IPPROTO_DONE) { 348 ipsecstat_inc(ipsec_idrops); 349 tdbp->tdb_idrops++; 350 } 351 tdb_unref(tdbp); > bluhm > > Index: net/if_bridge.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_bridge.c,v > retrieving revision 1.358 > diff -u -p -r1.358 if_bridge.c > --- net/if_bridge.c 11 Nov 2021 18:08:17 -0000 1.358 > +++ net/if_bridge.c 11 Nov 2021 23:24:31 -0000 > @@ -1577,10 +1577,12 @@ bridge_ipsec(struct ifnet *ifp, struct e > > prot = (*(tdb->tdb_xform->xf_input))(&m, tdb, hlen, > off); > + tdb_unref(tdb); > if (prot != IPPROTO_DONE) > ip_deliver(&m, &hlen, prot, af); > return (1); > } else { > + tdb_unref(tdb); > skiplookup: > /* XXX do an input policy lookup */ > return (0); > Index: net/if_pfsync.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_pfsync.c,v > retrieving revision 1.298 > diff -u -p -r1.298 if_pfsync.c > --- net/if_pfsync.c 11 Nov 2021 12:35:01 -0000 1.298 > +++ net/if_pfsync.c 11 Nov 2021 23:24:31 -0000 > @@ -1325,11 +1325,13 @@ pfsync_update_net_tdb(struct pfsync_tdb > /* Neither replay nor byte counter should ever decrease. */ > if (pt->rpl < tdb->tdb_rpl || > pt->cur_bytes < tdb->tdb_cur_bytes) { > + tdb_unref(tdb); > goto bad; > } > > tdb->tdb_rpl = pt->rpl; > tdb->tdb_cur_bytes = pt->cur_bytes; > + tdb_unref(tdb); > } > return; > > Index: net/pfkeyv2.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfkeyv2.c,v > retrieving revision 1.221 > diff -u -p -r1.221 pfkeyv2.c > --- net/pfkeyv2.c 25 Oct 2021 18:25:01 -0000 1.221 > +++ net/pfkeyv2.c 14 Nov 2021 21:34:20 -0000 > @@ -1044,7 +1044,8 @@ pfkeyv2_sa_flush(struct tdb *tdb, void * > if (!(*((u_int8_t *) satype_vp)) || > tdb->tdb_satype == *((u_int8_t *) satype_vp)) { > tdb_unlink_locked(tdb); > - tdb_free(tdb); > + tdb_unbundle(tdb); > + tdb_unref(tdb); > } > return (0); > } > @@ -1327,7 +1328,7 @@ pfkeyv2_send(struct socket *so, void *me > > if ((rval = pfkeyv2_get_proto_alg(newsa->tdb_satype, > &newsa->tdb_sproto, &alg))) { > - tdb_free(freeme); > + tdb_unref(freeme); > freeme = NULL; > NET_UNLOCK(); > goto ret; > @@ -1363,7 +1364,7 @@ pfkeyv2_send(struct socket *so, void *me > headers[SADB_X_EXT_DST_MASK], > headers[SADB_X_EXT_PROTOCOL], > headers[SADB_X_EXT_FLOW_TYPE]))) { > - tdb_free(freeme); > + tdb_unref(freeme); > freeme = NULL; > NET_UNLOCK(); > goto ret; > @@ -1386,7 +1387,7 @@ pfkeyv2_send(struct socket *so, void *me > rval = tdb_init(newsa, alg, &ii); > if (rval) { > rval = EINVAL; > - tdb_free(freeme); > + tdb_unref(freeme); > freeme = NULL; > NET_UNLOCK(); > goto ret; > @@ -1397,7 +1398,7 @@ pfkeyv2_send(struct socket *so, void *me > /* Delete old version of the SA, insert new one */ > tdb_delete(sa2); > puttdb((struct tdb *) freeme); > - sa2 = freeme = NULL; > + freeme = NULL; > } else { > /* > * The SA is already initialized, so we're only allowed > to > @@ -1503,7 +1504,7 @@ pfkeyv2_send(struct socket *so, void *me > newsa->tdb_satype = smsg->sadb_msg_satype; > if ((rval = pfkeyv2_get_proto_alg(newsa->tdb_satype, > &newsa->tdb_sproto, &alg))) { > - tdb_free(freeme); > + tdb_unref(freeme); > freeme = NULL; > NET_UNLOCK(); > goto ret; > @@ -1541,7 +1542,7 @@ pfkeyv2_send(struct socket *so, void *me > headers[SADB_X_EXT_DST_MASK], > headers[SADB_X_EXT_PROTOCOL], > headers[SADB_X_EXT_FLOW_TYPE]))) { > - tdb_free(freeme); > + tdb_unref(freeme); > freeme = NULL; > NET_UNLOCK(); > goto ret; > @@ -1564,7 +1565,7 @@ pfkeyv2_send(struct socket *so, void *me > rval = tdb_init(newsa, alg, &ii); > if (rval) { > rval = EINVAL; > - tdb_free(freeme); > + tdb_unref(freeme); > freeme = NULL; > NET_UNLOCK(); > goto ret; > @@ -1596,7 +1597,6 @@ pfkeyv2_send(struct socket *so, void *me > tdb_delete(sa2); > NET_UNLOCK(); > > - sa2 = NULL; > break; > > case SADB_X_ASKPOLICY: > @@ -1786,6 +1786,7 @@ pfkeyv2_send(struct socket *so, void *me > ssa->sadb_sa_spi, sunionp, > SADB_X_GETSPROTO(sa_proto->sadb_protocol_proto)); > if (tdb2 == NULL) { > + tdb_unref(tdb1); > rval = ESRCH; > NET_UNLOCK(); > goto ret; > @@ -1794,6 +1795,8 @@ pfkeyv2_send(struct socket *so, void *me > /* Detect cycles */ > for (tdb3 = tdb2; tdb3; tdb3 = tdb3->tdb_onext) > if (tdb3 == tdb1) { > + tdb_unref(tdb1); > + tdb_unref(tdb2); > rval = ESRCH; > NET_UNLOCK(); > goto ret; > @@ -1801,12 +1804,16 @@ pfkeyv2_send(struct socket *so, void *me > > /* Maintenance */ > if ((tdb1->tdb_onext) && > - (tdb1->tdb_onext->tdb_inext == tdb1)) > + (tdb1->tdb_onext->tdb_inext == tdb1)) { > + tdb_unref(tdb1->tdb_onext->tdb_inext); > tdb1->tdb_onext->tdb_inext = NULL; > + } > > if ((tdb2->tdb_inext) && > - (tdb2->tdb_inext->tdb_onext == tdb2)) > + (tdb2->tdb_inext->tdb_onext == tdb2)) { > + tdb_unref(tdb2->tdb_inext->tdb_onext); > tdb2->tdb_inext->tdb_onext = NULL; > + } > > /* Link them */ > tdb1->tdb_onext = tdb2; > @@ -2008,10 +2015,12 @@ pfkeyv2_send(struct socket *so, void *me > (caddr_t)&ipo->ipo_mask, rnh, > ipo->ipo_nodes, 0)) == NULL) { > /* Remove from linked list of policies on TDB */ > - if (ipo->ipo_tdb) > - > TAILQ_REMOVE(&ipo->ipo_tdb->tdb_policy_head, > + if (ipo->ipo_tdb != NULL) { > + TAILQ_REMOVE( > + &ipo->ipo_tdb->tdb_policy_head, > ipo, ipo_tdb_next); > - > + tdb_unref(ipo->ipo_tdb); > + } > if (ipo->ipo_ids) > ipsp_ids_free(ipo->ipo_ids); > pool_put(&ipsec_policy_pool, ipo); > @@ -2127,6 +2136,10 @@ realret: > free(message, M_PFKEY, len); > > free(sa1, M_PFKEY, sizeof(*sa1)); > + > + NET_LOCK(); > + tdb_unref(sa2); > + NET_UNLOCK(); > > return (rval); > } > Index: netinet/ip_ipsp.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.c,v > retrieving revision 1.249 > diff -u -p -r1.249 ip_ipsp.c > --- netinet/ip_ipsp.c 27 Oct 2021 16:58:44 -0000 1.249 > +++ netinet/ip_ipsp.c 13 Nov 2021 15:59:39 -0000 > @@ -297,9 +297,10 @@ reserve_spi(u_int rdomain, u_int32_t ssp > > /* Check whether we're using this SPI already. */ > exists = gettdb(rdomain, spi, dst, sproto); > - if (exists) > + if (exists != NULL) { > + tdb_unref(exists); > continue; > - > + } > > tdbp->tdb_spi = spi; > memcpy(&tdbp->tdb_dst.sa, &dst->sa, dst->sa.sa_len); > @@ -351,6 +352,7 @@ gettdb_dir(u_int rdomain, u_int32_t spi, > !memcmp(&tdbp->tdb_dst, dst, dst->sa.sa_len)) > break; > > + tdb_ref(tdbp); > mtx_leave(&tdb_sadb_mtx); > return tdbp; > } > @@ -383,6 +385,7 @@ gettdbbysrcdst_dir(u_int rdomain, u_int3 > break; > > if (tdbp != NULL) { > + tdb_ref(tdbp); > mtx_leave(&tdb_sadb_mtx); > return tdbp; > } > @@ -402,6 +405,7 @@ gettdbbysrcdst_dir(u_int rdomain, u_int3 > tdbp->tdb_src.sa.sa_family == AF_UNSPEC) > break; > > + tdb_ref(tdbp); > mtx_leave(&tdb_sadb_mtx); > return tdbp; > } > @@ -469,6 +473,7 @@ gettdbbydst(u_int rdomain, union sockadd > break; > } > > + tdb_ref(tdbp); > mtx_leave(&tdb_sadb_mtx); > return tdbp; > } > @@ -499,6 +504,7 @@ gettdbbysrc(u_int rdomain, union sockadd > break; > } > > + tdb_ref(tdbp); > mtx_leave(&tdb_sadb_mtx); > return tdbp; > } > @@ -811,12 +817,55 @@ tdb_unlink_locked(struct tdb *tdbp) > } > > void > +tdb_unbundle(struct tdb *tdbp) > +{ > + if (tdbp->tdb_onext) { > + if (tdbp->tdb_onext->tdb_inext == tdbp) { > + tdb_unref(tdbp); /* to us */ > + tdbp->tdb_onext->tdb_inext = NULL; > + } > + tdb_unref(tdbp->tdb_onext); /* to other */ > + tdbp->tdb_onext = NULL; > + } > + if (tdbp->tdb_inext) { > + if (tdbp->tdb_inext->tdb_onext == tdbp) { > + tdb_unref(tdbp); /* to us */ > + tdbp->tdb_inext->tdb_onext = NULL; > + } > + tdb_unref(tdbp->tdb_inext); /* to other */ > + tdbp->tdb_inext = NULL; > + } > +} > + > +struct tdb * > +tdb_ref(struct tdb *tdb) > +{ > + if (tdb == NULL) > + return NULL; > + refcnt_take(&tdb->tdb_refcnt); > + return tdb; > +} > + > +void > +tdb_unref(struct tdb *tdb) > +{ > + if (tdb == NULL) > + return; > + if (refcnt_rele(&tdb->tdb_refcnt) == 0) > + return; > + tdb_free(tdb); > +} > + > +void > tdb_delete(struct tdb *tdbp) > { > NET_ASSERT_LOCKED(); > > tdb_unlink(tdbp); > - tdb_free(tdbp); > + /* release tdb_onext/tdb_inext references */ > + tdb_unbundle(tdbp); > + /* release the reference for tdb_unlink() */ > + tdb_unref(tdbp); > } > > /* > @@ -831,6 +880,7 @@ tdb_alloc(u_int rdomain) > > tdbp = pool_get(&tdb_pool, PR_WAITOK | PR_ZERO); > > + refcnt_init(&tdbp->tdb_refcnt); > TAILQ_INIT(&tdbp->tdb_policy_head); > > /* Record establishment time. */ > @@ -867,9 +917,9 @@ tdb_free(struct tdb *tdbp) > #endif > > /* Cleanup SPD references. */ > - for (ipo = TAILQ_FIRST(&tdbp->tdb_policy_head); ipo; > - ipo = TAILQ_FIRST(&tdbp->tdb_policy_head)) { > + while ((ipo = TAILQ_FIRST(&tdbp->tdb_policy_head)) != NULL) { > TAILQ_REMOVE(&tdbp->tdb_policy_head, ipo, ipo_tdb_next); > + tdb_unref(ipo->ipo_tdb); > ipo->ipo_tdb = NULL; > ipo->ipo_last_searched = 0; /* Force a re-search. */ > } > @@ -885,12 +935,6 @@ tdb_free(struct tdb *tdbp) > tdbp->tdb_tag = 0; > } > #endif > - > - if ((tdbp->tdb_onext) && (tdbp->tdb_onext->tdb_inext == tdbp)) > - tdbp->tdb_onext->tdb_inext = NULL; > - > - if ((tdbp->tdb_inext) && (tdbp->tdb_inext->tdb_onext == tdbp)) > - tdbp->tdb_inext->tdb_onext = NULL; > > /* Remove expiration timeouts. */ > tdbp->tdb_flags &= ~(TDBF_FIRSTUSE | TDBF_SOFT_FIRSTUSE | TDBF_TIMER | > Index: netinet/ip_ipsp.h > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.h,v > retrieving revision 1.219 > diff -u -p -r1.219 ip_ipsp.h > --- netinet/ip_ipsp.h 25 Oct 2021 18:25:01 -0000 1.219 > +++ netinet/ip_ipsp.h 13 Nov 2021 15:59:19 -0000 > @@ -322,6 +322,8 @@ struct tdb { /* tunnel > descriptor blo > struct tdb *tdb_inext; > struct tdb *tdb_onext; > > + struct refcnt tdb_refcnt; > + > const struct xformsw *tdb_xform; /* Transform to use */ > const struct enc_xform *tdb_encalgxform; /* Enc algorithm */ > const struct auth_hash *tdb_authalgxform; /* Auth algorithm */ > @@ -555,10 +557,13 @@ struct tdb *gettdbbysrcdst_dir(u_int, u_ > void puttdb(struct tdb *); > void tdb_delete(struct tdb *); > struct tdb *tdb_alloc(u_int); > +struct tdb *tdb_ref(struct tdb *); > +void tdb_unref(struct tdb *); > void tdb_free(struct tdb *); > int tdb_init(struct tdb *, u_int16_t, struct ipsecinit *); > void tdb_unlink(struct tdb *); > void tdb_unlink_locked(struct tdb *); > +void tdb_unbundle(struct tdb *); > int tdb_walk(u_int, int (*)(struct tdb *, void *, int), void *); > > /* XF_IP4 */ > Index: netinet/ip_spd.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_spd.c,v > retrieving revision 1.104 > diff -u -p -r1.104 ip_spd.c > --- netinet/ip_spd.c 8 Jul 2021 16:39:55 -0000 1.104 > +++ netinet/ip_spd.c 11 Nov 2021 23:24:31 -0000 > @@ -368,9 +368,11 @@ ipsp_spd_lookup(struct mbuf *m, int af, > } > > /* Do we have a cached entry ? If so, check if it's still valid. */ > - if ((ipo->ipo_tdb) && (ipo->ipo_tdb->tdb_flags & TDBF_INVALID)) { > + if (ipo->ipo_tdb != NULL && > + (ipo->ipo_tdb->tdb_flags & TDBF_INVALID)) { > TAILQ_REMOVE(&ipo->ipo_tdb->tdb_policy_head, ipo, > ipo_tdb_next); > + tdb_unref(ipo->ipo_tdb); > ipo->ipo_tdb = NULL; > } > > @@ -398,7 +400,7 @@ ipsp_spd_lookup(struct mbuf *m, int af, > ids = ipsp_ids_lookup(ipsecflowinfo); > > /* Check that the cached TDB (if present), is appropriate. */ > - if (ipo->ipo_tdb) { > + if (ipo->ipo_tdb != NULL) { > if ((ipo->ipo_last_searched <= ipsec_last_added) || > (ipo->ipo_sproto != ipo->ipo_tdb->tdb_sproto) || > memcmp(dignore ? &sdst : &ipo->ipo_dst, > @@ -420,6 +422,7 @@ ipsp_spd_lookup(struct mbuf *m, int af, > /* Cached TDB was not good. */ > TAILQ_REMOVE(&ipo->ipo_tdb->tdb_policy_head, ipo, > ipo_tdb_next); > + tdb_unref(ipo->ipo_tdb); > ipo->ipo_tdb = NULL; > ipo->ipo_last_searched = 0; > } > @@ -439,14 +442,14 @@ ipsp_spd_lookup(struct mbuf *m, int af, > ipo->ipo_last_searched = getuptime(); > > /* Find an appropriate SA from the existing ones. */ > - ipo->ipo_tdb = > - gettdbbydst(rdomain, > - dignore ? &sdst : &ipo->ipo_dst, > - ipo->ipo_sproto, > - ids ? ids: ipo->ipo_ids, > - &ipo->ipo_addr, &ipo->ipo_mask); > - if (ipo->ipo_tdb) { > - > TAILQ_INSERT_TAIL(&ipo->ipo_tdb->tdb_policy_head, > + ipo->ipo_tdb = gettdbbydst(rdomain, > + dignore ? &sdst : &ipo->ipo_dst, > + ipo->ipo_sproto, ids ? ids: ipo->ipo_ids, > + &ipo->ipo_addr, &ipo->ipo_mask); > + if (ipo->ipo_tdb != NULL) { > + /* gettdbbydst() has already refcounted tdb */ > + TAILQ_INSERT_TAIL( > + &ipo->ipo_tdb->tdb_policy_head, > ipo, ipo_tdb_next); > *error = 0; > return ipsp_spd_inp(m, af, hlen, error, > @@ -520,10 +523,12 @@ ipsp_spd_lookup(struct mbuf *m, int af, > goto nomatchin; > > /* Add it to the cache. */ > - if (ipo->ipo_tdb) > + if (ipo->ipo_tdb != NULL) { > TAILQ_REMOVE(&ipo->ipo_tdb->tdb_policy_head, > ipo, ipo_tdb_next); > - ipo->ipo_tdb = tdbp; > + tdb_unref(ipo->ipo_tdb); > + } > + ipo->ipo_tdb = tdb_ref(tdbp); > TAILQ_INSERT_TAIL(&tdbp->tdb_policy_head, ipo, > ipo_tdb_next); > *error = 0; > @@ -535,7 +540,7 @@ ipsp_spd_lookup(struct mbuf *m, int af, > } > > /* Check whether cached entry applies. */ > - if (ipo->ipo_tdb) { > + if (ipo->ipo_tdb != NULL) { > /* > * We only need to check that the correct > * security protocol and security gateway are > @@ -551,8 +556,9 @@ ipsp_spd_lookup(struct mbuf *m, int af, > /* Not applicable, unlink. */ > TAILQ_REMOVE(&ipo->ipo_tdb->tdb_policy_head, ipo, > ipo_tdb_next); > - ipo->ipo_last_searched = 0; > + tdb_unref(ipo->ipo_tdb); > ipo->ipo_tdb = NULL; > + ipo->ipo_last_searched = 0; > } > > /* Find whether there exists an appropriate SA. */ > @@ -560,14 +566,16 @@ ipsp_spd_lookup(struct mbuf *m, int af, > if (dignore == 0) > ipo->ipo_last_searched = getuptime(); > > - ipo->ipo_tdb = > - gettdbbysrc(rdomain, > - dignore ? &ssrc : &ipo->ipo_dst, > - ipo->ipo_sproto, ipo->ipo_ids, > - &ipo->ipo_addr, &ipo->ipo_mask); > - if (ipo->ipo_tdb) > - > TAILQ_INSERT_TAIL(&ipo->ipo_tdb->tdb_policy_head, > + ipo->ipo_tdb = gettdbbysrc(rdomain, > + dignore ? &ssrc : &ipo->ipo_dst, > + ipo->ipo_sproto, ipo->ipo_ids, > + &ipo->ipo_addr, &ipo->ipo_mask); > + if (ipo->ipo_tdb != NULL) { > + /* gettdbbysrc() has already refcounted tdb */ > + TAILQ_INSERT_TAIL( > + &ipo->ipo_tdb->tdb_policy_head, > ipo, ipo_tdb_next); > + } > } > skipinputsearch: > > @@ -637,9 +645,12 @@ ipsec_delete_policy(struct ipsec_policy > rn_delete(&ipo->ipo_addr, &ipo->ipo_mask, rnh, rn) == NULL) > return (ESRCH); > > - if (ipo->ipo_tdb != NULL) > + if (ipo->ipo_tdb != NULL) { > TAILQ_REMOVE(&ipo->ipo_tdb->tdb_policy_head, ipo, > ipo_tdb_next); > + tdb_unref(ipo->ipo_tdb); > + ipo->ipo_tdb = NULL; > + } > > while ((ipa = TAILQ_FIRST(&ipo->ipo_acquires)) != NULL) > ipsp_delete_acquire(ipa); > Index: netinet/ipsec_input.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ipsec_input.c,v > retrieving revision 1.191 > diff -u -p -r1.191 ipsec_input.c > --- netinet/ipsec_input.c 11 Nov 2021 18:08:18 -0000 1.191 > +++ netinet/ipsec_input.c 11 Nov 2021 23:24:31 -0000 > @@ -348,6 +348,7 @@ ipsec_common_input(struct mbuf **mp, int > ipsecstat_inc(ipsec_idrops); > tdbp->tdb_idrops++; > } > + tdb_unref(tdbp); > return prot; > > drop: > @@ -355,6 +356,7 @@ ipsec_common_input(struct mbuf **mp, int > ipsecstat_inc(ipsec_idrops); > if (tdbp != NULL) > tdbp->tdb_idrops++; > + tdb_unref(tdbp); > return IPPROTO_DONE; > } > > @@ -937,6 +939,7 @@ ipsec_common_ctlinput(u_int rdomain, int > tdbp = gettdb_rev(rdomain, spi, (union sockaddr_union *)&dst, > proto); > ipsec_set_mtu(tdbp, mtu); > + tdb_unref(tdbp); > } > } > > @@ -944,7 +947,7 @@ void > udpencap_ctlinput(int cmd, struct sockaddr *sa, u_int rdomain, void *v) > { > struct ip *ip = v; > - struct tdb *tdbp; > + struct tdb *tdbp, *first; > struct icmp *icp; > u_int32_t mtu; > struct sockaddr_in dst, src; > @@ -973,10 +976,9 @@ udpencap_ctlinput(int cmd, struct sockad > src.sin_addr.s_addr = ip->ip_src.s_addr; > su_src = (union sockaddr_union *)&src; > > - tdbp = gettdbbysrcdst_rev(rdomain, 0, su_src, su_dst, > - IPPROTO_ESP); > + first = gettdbbysrcdst_rev(rdomain, 0, su_src, su_dst, IPPROTO_ESP); > > - for (; tdbp != NULL; tdbp = tdbp->tdb_snext) { > + for (tdbp = first; tdbp != NULL; tdbp = tdbp->tdb_snext) { > if (tdbp->tdb_sproto == IPPROTO_ESP && > ((tdbp->tdb_flags & (TDBF_INVALID|TDBF_UDPENCAP)) == > TDBF_UDPENCAP) && > @@ -985,6 +987,7 @@ udpencap_ctlinput(int cmd, struct sockad > ipsec_set_mtu(tdbp, mtu); > } > } > + tdb_unref(first); > } > > void > @@ -1070,6 +1073,7 @@ ipsec_forward_check(struct mbuf *m, int > } else > tdb = NULL; > ipsp_spd_lookup(m, af, hlen, &error, IPSP_DIRECTION_IN, tdb, NULL, 0); > + tdb_unref(tdb); > > return error; > } > @@ -1142,6 +1146,7 @@ ipsec_local_check(struct mbuf *m, int hl > tdb = NULL; > ipsp_spd_lookup(m, af, hlen, &error, IPSP_DIRECTION_IN, > tdb, NULL, 0); > + tdb_unref(tdb); > > return error; > } > Index: netinet/ipsec_output.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ipsec_output.c,v > retrieving revision 1.91 > diff -u -p -r1.91 ipsec_output.c > --- netinet/ipsec_output.c 23 Oct 2021 15:42:35 -0000 1.91 > +++ netinet/ipsec_output.c 12 Nov 2021 21:03:24 -0000 > @@ -388,6 +388,7 @@ ipsp_process_done(struct mbuf *m, struct > #ifdef INET6 > struct ip6_hdr *ip6; > #endif /* INET6 */ > + struct tdb *tdbo; > struct tdb_ident *tdbi; > struct m_tag *mtag; > int roff, error; > @@ -501,9 +502,13 @@ ipsp_process_done(struct mbuf *m, struct > tdb->tdb_obytes += m->m_pkthdr.len; > > /* If there's another (bundled) TDB to apply, do so. */ > - if (tdb->tdb_onext) > - return ipsp_process_packet(m, tdb->tdb_onext, > + tdbo = tdb_ref(tdb->tdb_onext); > + if (tdbo != NULL) { > + error = ipsp_process_packet(m, tdbo, > tdb->tdb_dst.sa.sa_family, 0); > + tdb_unref(tdbo); > + return error; > + } > > #if NPF > 0 > /* Add pf tag if requested. */ > @@ -615,13 +620,16 @@ ipsec_adjust_mtu(struct mbuf *m, u_int32 > if (tdbp == NULL) > break; > > - if ((adjust = ipsec_hdrsz(tdbp)) == -1) > + if ((adjust = ipsec_hdrsz(tdbp)) == -1) { > + tdb_unref(tdbp); > break; > + } > > mtu -= adjust; > tdbp->tdb_mtu = mtu; > tdbp->tdb_mtutimeout = gettime() + ip_mtudisc_timeout; > DPRINTF("spi %08x mtu %d adjust %ld mbuf %p", > ntohl(tdbp->tdb_spi), tdbp->tdb_mtu, adjust, m); > + tdb_unref(tdbp); > } > } > Index: netinet/tcp_input.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v > retrieving revision 1.370 > diff -u -p -r1.370 tcp_input.c > --- netinet/tcp_input.c 9 Aug 2021 17:03:08 -0000 1.370 > +++ netinet/tcp_input.c 11 Nov 2021 23:24:31 -0000 > @@ -380,12 +380,6 @@ tcp_input(struct mbuf **mp, int *offp, i > #ifdef INET6 > struct ip6_hdr *ip6 = NULL; > #endif /* INET6 */ > -#ifdef IPSEC > - struct m_tag *mtag; > - struct tdb_ident *tdbi; > - struct tdb *tdb; > - int error; > -#endif /* IPSEC */ > #ifdef TCP_ECN > u_char iptos; > #endif > @@ -571,16 +565,22 @@ findpcb: > } > #ifdef IPSEC > if (ipsec_in_use) { > + struct m_tag *mtag; > + struct tdb *tdb = NULL; > + int error; > + > /* Find most recent IPsec tag */ > mtag = m_tag_find(m, PACKET_TAG_IPSEC_IN_DONE, NULL); > if (mtag != NULL) { > + struct tdb_ident *tdbi; > + > tdbi = (struct tdb_ident *)(mtag + 1); > tdb = gettdb(tdbi->rdomain, tdbi->spi, > &tdbi->dst, tdbi->proto); > - } else > - tdb = NULL; > + } > ipsp_spd_lookup(m, af, iphlen, &error, IPSP_DIRECTION_IN, > tdb, inp, 0); > + tdb_unref(tdb); > if (error) { > tcpstat_inc(tcps_rcvnosec); > goto drop; > @@ -2197,7 +2197,7 @@ tcp_dooptions(struct tcpcb *tp, u_char * > continue; > > if (sigp && timingsafe_bcmp(sigp, cp + 2, 16)) > - return (-1); > + goto bad; > > sigp = cp + 2; > break; > @@ -2248,7 +2248,7 @@ tcp_dooptions(struct tcpcb *tp, u_char * > > if ((sigp ? TF_SIGNATURE : 0) ^ (tp->t_flags & TF_SIGNATURE)) { > tcpstat_inc(tcps_rcvbadsig); > - return (-1); > + goto bad; > } > > if (sigp) { > @@ -2256,22 +2256,30 @@ tcp_dooptions(struct tcpcb *tp, u_char * > > if (tdb == NULL) { > tcpstat_inc(tcps_rcvbadsig); > - return (-1); > + goto bad; > } > > if (tcp_signature(tdb, tp->pf, m, th, iphlen, 1, sig) < 0) > - return (-1); > + goto bad; > > if (timingsafe_bcmp(sig, sigp, 16)) { > tcpstat_inc(tcps_rcvbadsig); > - return (-1); > + goto bad; > } > > tcpstat_inc(tcps_rcvgoodsig); > } > + > + tdb_unref(tdb); > #endif /* TCP_SIGNATURE */ > > return (0); > + > + bad: > +#ifdef TCP_SIGNATURE > + tdb_unref(tdb); > +#endif /* TCP_SIGNATURE */ > + return (-1); > } > > u_long > @@ -4056,8 +4064,10 @@ syn_cache_respond(struct syn_cache *sc, > if (tcp_signature(tdb, sc->sc_src.sa.sa_family, m, th, > hlen, 0, optp) < 0) { > m_freem(m); > + tdb_unref(tdb); > return (EINVAL); > } > + tdb_unref(tdb); > optp += 16; > > /* Pad options list to the next 32 bit boundary and > Index: netinet/tcp_output.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_output.c,v > retrieving revision 1.130 > diff -u -p -r1.130 tcp_output.c > --- netinet/tcp_output.c 8 Feb 2021 19:37:15 -0000 1.130 > +++ netinet/tcp_output.c 12 Nov 2021 21:01:58 -0000 > @@ -879,8 +879,10 @@ send: > if (tcp_signature(tdb, tp->pf, m, th, iphlen, 0, > mtod(m, caddr_t) + hdrlen - optlen + sigoff) < 0) { > m_freem(m); > + tdb_unref(tdb); > return (EINVAL); > } > + tdb_unref(tdb); > } > #endif /* TCP_SIGNATURE */ > > Index: netinet/udp_usrreq.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/udp_usrreq.c,v > retrieving revision 1.264 > diff -u -p -r1.264 udp_usrreq.c > --- netinet/udp_usrreq.c 11 Nov 2021 18:08:18 -0000 1.264 > +++ netinet/udp_usrreq.c 12 Nov 2021 21:01:58 -0000 > @@ -514,11 +514,13 @@ udp_input(struct mbuf **mp, int *offp, i > IPSP_DIRECTION_IN, tdb, inp, 0); > if (error) { > udpstat_inc(udps_nosec); > + tdb_unref(tdb); > goto bad; > } > /* create ipsec options while we know that tdb cannot be > modified */ > if (tdb && tdb->tdb_ids) > ipsecflowinfo = tdb->tdb_ids->id_flow; > + tdb_unref(tdb); > } > #endif /*IPSEC */ > >