On Thu, Nov 25, 2021 at 09:52:54AM +0100, Alexander Bluhm wrote: > On Sat, Nov 13, 2021 at 06:04:07PM +0100, Alexander Bluhm wrote: > > When testing, please check for tdb leaks. > > The diff below was running on my performance setup for more than > 10 hours. iked SA lifetime was about 10 seconds. ipsecctl -F; > vmstat -m showed no leak. Running regress passed. > > Hrvoje is also testing this diff. So I would recommend to commit > what we have after positive feedback from Hrvoje and some OKs. > > Things like removing TDBF_DELETED flag from tobhe@ or concerns from > mvs@ that we need more ref count protection will be added later. > > ok?
ok mvs@ > > 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 24 Nov 2021 13:24:23 -0000 > @@ -1567,20 +1567,28 @@ bridge_ipsec(struct ifnet *ifp, struct e > tdb->tdb_xform != NULL) { > if (tdb->tdb_first_use == 0) { > tdb->tdb_first_use = gettime(); > - if (tdb->tdb_flags & TDBF_FIRSTUSE) > - timeout_add_sec(&tdb->tdb_first_tmo, > - tdb->tdb_exp_first_use); > - if (tdb->tdb_flags & TDBF_SOFT_FIRSTUSE) > - timeout_add_sec(&tdb->tdb_sfirst_tmo, > - tdb->tdb_soft_first_use); > + if (tdb->tdb_flags & TDBF_FIRSTUSE) { > + if (timeout_add_sec( > + &tdb->tdb_first_tmo, > + tdb->tdb_exp_first_use)) > + tdb_ref(tdb); > + } > + if (tdb->tdb_flags & TDBF_SOFT_FIRSTUSE) { > + if (timeout_add_sec( > + &tdb->tdb_sfirst_tmo, > + tdb->tdb_soft_first_use)) > + tdb_ref(tdb); > + } > } > > 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 24 Nov 2021 13:24:23 -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 24 Nov 2021 14:15:29 -0000 > @@ -1043,8 +1043,17 @@ pfkeyv2_sa_flush(struct tdb *tdb, void * > { > if (!(*((u_int8_t *) satype_vp)) || > tdb->tdb_satype == *((u_int8_t *) satype_vp)) { > + /* keep in sync with tdb_delete() */ > + NET_ASSERT_LOCKED(); > + > + if (tdb->tdb_flags & TDBF_DELETED) > + return (0); > + tdb->tdb_flags |= TDBF_DELETED; > + > tdb_unlink_locked(tdb); > - tdb_free(tdb); > + tdb_unbundle(tdb); > + tdb_deltimeouts(tdb); > + tdb_unref(tdb); > } > return (0); > } > @@ -1327,7 +1336,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 +1372,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 +1395,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 +1406,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 +1512,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 +1550,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 +1573,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 +1605,6 @@ pfkeyv2_send(struct socket *so, void *me > tdb_delete(sa2); > NET_UNLOCK(); > > - sa2 = NULL; > break; > > case SADB_X_ASKPOLICY: > @@ -1786,6 +1794,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 +1803,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 +1812,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 +2023,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 +2144,10 @@ realret: > free(message, M_PFKEY, len); > > free(sa1, M_PFKEY, sizeof(*sa1)); > + > + NET_LOCK(); > + tdb_unref(sa2); > + NET_UNLOCK(); > > return (rval); > } > Index: net/pfkeyv2_convert.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfkeyv2_convert.c,v > retrieving revision 1.75 > diff -u -p -r1.75 pfkeyv2_convert.c > --- net/pfkeyv2_convert.c 22 Oct 2021 12:30:53 -0000 1.75 > +++ net/pfkeyv2_convert.c 24 Nov 2021 13:24:23 -0000 > @@ -299,8 +299,9 @@ import_lifetime(struct tdb *tdb, struct > if ((tdb->tdb_exp_timeout = > sadb_lifetime->sadb_lifetime_addtime) != 0) { > tdb->tdb_flags |= TDBF_TIMER; > - timeout_add_sec(&tdb->tdb_timer_tmo, > - tdb->tdb_exp_timeout); > + if (timeout_add_sec(&tdb->tdb_timer_tmo, > + tdb->tdb_exp_timeout)) > + tdb_ref(tdb); > } else > tdb->tdb_flags &= ~TDBF_TIMER; > > @@ -327,8 +328,9 @@ import_lifetime(struct tdb *tdb, struct > if ((tdb->tdb_soft_timeout = > sadb_lifetime->sadb_lifetime_addtime) != 0) { > tdb->tdb_flags |= TDBF_SOFT_TIMER; > - timeout_add_sec(&tdb->tdb_stimer_tmo, > - tdb->tdb_soft_timeout); > + if (timeout_add_sec(&tdb->tdb_stimer_tmo, > + tdb->tdb_soft_timeout)) > + tdb_ref(tdb); > } else > tdb->tdb_flags &= ~TDBF_SOFT_TIMER; > > Index: netinet/ip_ipsp.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.c,v > retrieving revision 1.253 > diff -u -p -r1.253 ip_ipsp.c > --- netinet/ip_ipsp.c 21 Nov 2021 16:17:48 -0000 1.253 > +++ netinet/ip_ipsp.c 24 Nov 2021 14:12:44 -0000 > @@ -85,7 +85,6 @@ void tdb_hashstats(void); > #endif > > int tdb_rehash(void); > -void tdb_reaper(void *); > void tdb_timeout(void *); > void tdb_firstuse(void *); > void tdb_soft_timeout(void *); > @@ -297,9 +296,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); > @@ -314,8 +314,9 @@ reserve_spi(u_int rdomain, u_int32_t ssp > if (ipsec_keep_invalid > 0) { > tdbp->tdb_flags |= TDBF_TIMER; > tdbp->tdb_exp_timeout = ipsec_keep_invalid; > - timeout_add_sec(&tdbp->tdb_timer_tmo, > - ipsec_keep_invalid); > + if (timeout_add_sec(&tdbp->tdb_timer_tmo, > + ipsec_keep_invalid)) > + tdb_ref(tdbp); > } > #endif > > @@ -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; > } > @@ -548,6 +554,7 @@ tdb_printit(void *addr, int full, int (* > DUMP(inext, "%p"); > DUMP(onext, "%p"); > DUMP(xform, "%p"); > + pr("%18s: %d\n", "refcnt", tdb->tdb_refcnt.refs); > DUMP(encalgxform, "%p"); > DUMP(authalgxform, "%p"); > DUMP(compalgxform, "%p"); > @@ -607,6 +614,7 @@ tdb_printit(void *addr, int full, int (* > pr(" %s", ipsp_address(&tdb->tdb_src, buf, sizeof(buf))); > pr("->%s", ipsp_address(&tdb->tdb_dst, buf, sizeof(buf))); > pr(":%d", tdb->tdb_sproto); > + pr(" #%d", tdb->tdb_refcnt.refs); > pr(" %08x\n", tdb->tdb_flags); > } > } > @@ -658,6 +666,8 @@ tdb_timeout(void *v) > } > tdb_delete(tdb); > } > + /* decrement refcount of the timeout argument */ > + tdb_unref(tdb); > NET_UNLOCK(); > } > > @@ -675,6 +685,8 @@ tdb_firstuse(void *v) > } > tdb_delete(tdb); > } > + /* decrement refcount of the timeout argument */ > + tdb_unref(tdb); > NET_UNLOCK(); > } > > @@ -689,6 +701,8 @@ tdb_soft_timeout(void *v) > pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT); > tdb->tdb_flags &= ~TDBF_SOFT_TIMER; > } > + /* decrement refcount of the timeout argument */ > + tdb_unref(tdb); > NET_UNLOCK(); > } > > @@ -704,6 +718,8 @@ tdb_soft_firstuse(void *v) > pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT); > tdb->tdb_flags &= ~TDBF_SOFT_FIRSTUSE; > } > + /* decrement refcount of the timeout argument */ > + tdb_unref(tdb); > NET_UNLOCK(); > } > > @@ -894,12 +910,75 @@ tdb_unlink_locked(struct tdb *tdbp) > } > > void > +tdb_unbundle(struct tdb *tdbp) > +{ > + if (tdbp->tdb_onext != NULL) { > + 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 != NULL) { > + 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; > + } > +} > + > +void > +tdb_deltimeouts(struct tdb *tdbp) > +{ > + if (timeout_del(&tdbp->tdb_timer_tmo)) > + tdb_unref(tdbp); > + if (timeout_del(&tdbp->tdb_first_tmo)) > + tdb_unref(tdbp); > + if (timeout_del(&tdbp->tdb_stimer_tmo)) > + tdb_unref(tdbp); > + if (timeout_del(&tdbp->tdb_sfirst_tmo)) > + tdb_unref(tdbp); > +} > + > +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) > { > + /* keep in sync with pfkeyv2_sa_flush() */ > NET_ASSERT_LOCKED(); > > + if (tdbp->tdb_flags & TDBF_DELETED) > + return; > + tdbp->tdb_flags |= TDBF_DELETED; > + > tdb_unlink(tdbp); > - tdb_free(tdbp); > + /* release tdb_onext/tdb_inext references */ > + tdb_unbundle(tdbp); > + /* delete timeouts and release references */ > + tdb_deltimeouts(tdbp); > + /* release the reference for tdb_unlink() */ > + tdb_unref(tdbp); > } > > /* > @@ -914,6 +993,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. */ > @@ -950,9 +1030,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. */ > } > @@ -969,28 +1049,16 @@ tdb_free(struct tdb *tdbp) > } > #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; > + KASSERT(tdbp->tdb_onext == NULL); > + KASSERT(tdbp->tdb_inext == NULL); > > /* Remove expiration timeouts. */ > tdbp->tdb_flags &= ~(TDBF_FIRSTUSE | TDBF_SOFT_FIRSTUSE | TDBF_TIMER | > TDBF_SOFT_TIMER); > - timeout_del(&tdbp->tdb_timer_tmo); > - timeout_del(&tdbp->tdb_first_tmo); > - timeout_del(&tdbp->tdb_stimer_tmo); > - timeout_del(&tdbp->tdb_sfirst_tmo); > - > - timeout_set_proc(&tdbp->tdb_timer_tmo, tdb_reaper, tdbp); > - timeout_add(&tdbp->tdb_timer_tmo, 0); > -} > - > -void > -tdb_reaper(void *xtdbp) > -{ > - struct tdb *tdbp = xtdbp; > + KASSERT(timeout_pending(&tdbp->tdb_timer_tmo) == 0); > + KASSERT(timeout_pending(&tdbp->tdb_first_tmo) == 0); > + KASSERT(timeout_pending(&tdbp->tdb_stimer_tmo) == 0); > + KASSERT(timeout_pending(&tdbp->tdb_sfirst_tmo) == 0); > > pool_put(&tdb_pool, tdbp); > } > Index: netinet/ip_ipsp.h > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.h,v > retrieving revision 1.221 > diff -u -p -r1.221 ip_ipsp.h > --- netinet/ip_ipsp.h 21 Nov 2021 16:17:48 -0000 1.221 > +++ netinet/ip_ipsp.h 24 Nov 2021 14:12:44 -0000 > @@ -324,6 +324,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 */ > @@ -335,6 +337,7 @@ struct tdb { /* tunnel > descriptor blo > #define TDBF_ALLOCATIONS 0x00008 /* Check the flows counters */ > #define TDBF_INVALID 0x00010 /* This SPI is not valid > yet/anymore */ > #define TDBF_FIRSTUSE 0x00020 /* Expire after first use */ > +#define TDBF_DELETED 0x00040 /* This TDB has already been > deleted */ > #define TDBF_SOFT_TIMER 0x00080 /* Soft expiration */ > #define TDBF_SOFT_BYTES 0x00100 /* Soft expiration */ > #define TDBF_SOFT_ALLOCATIONS 0x00200 /* Soft expiration */ > @@ -349,7 +352,7 @@ struct tdb { /* tunnel > descriptor blo > > #define TDBF_BITS ("\20" \ > "\1UNIQUE\2TIMER\3BYTES\4ALLOCATIONS" \ > - "\5INVALID\6FIRSTUSE\10SOFT_TIMER" \ > + "\5INVALID\6FIRSTUSE\7DELETED\10SOFT_TIMER" \ > "\11SOFT_BYTES\12SOFT_ALLOCATIONS\13SOFT_FIRSTUSE\14PFS" \ > "\15TUNNELING" \ > "\21USEDTUNNEL\22UDPENCAP\23PFSYNC\24PFSYNC_RPL" \ > @@ -564,10 +567,14 @@ 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 *); > +void tdb_deltimeouts(struct tdb *); > int tdb_walk(u_int, int (*)(struct tdb *, void *, int), void *); > void tdb_printit(void *, int, int (*)(const char *, ...)); > > 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 24 Nov 2021 13:24:23 -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.192 > diff -u -p -r1.192 ipsec_input.c > --- netinet/ipsec_input.c 21 Nov 2021 02:54:56 -0000 1.192 > +++ netinet/ipsec_input.c 24 Nov 2021 13:24:23 -0000 > @@ -328,12 +328,16 @@ ipsec_common_input(struct mbuf **mp, int > /* Register first use, setup expiration timer. */ > if (tdbp->tdb_first_use == 0) { > tdbp->tdb_first_use = gettime(); > - if (tdbp->tdb_flags & TDBF_FIRSTUSE) > - timeout_add_sec(&tdbp->tdb_first_tmo, > - tdbp->tdb_exp_first_use); > - if (tdbp->tdb_flags & TDBF_SOFT_FIRSTUSE) > - timeout_add_sec(&tdbp->tdb_sfirst_tmo, > - tdbp->tdb_soft_first_use); > + if (tdbp->tdb_flags & TDBF_FIRSTUSE) { > + if (timeout_add_sec(&tdbp->tdb_first_tmo, > + tdbp->tdb_exp_first_use)) > + tdb_ref(tdbp); > + } > + if (tdbp->tdb_flags & TDBF_SOFT_FIRSTUSE) { > + if (timeout_add_sec(&tdbp->tdb_sfirst_tmo, > + tdbp->tdb_soft_first_use)) > + tdb_ref(tdbp); > + } > } > > tdbp->tdb_ipackets++; > @@ -348,6 +352,7 @@ ipsec_common_input(struct mbuf **mp, int > ipsecstat_inc(ipsec_idrops); > tdbp->tdb_idrops++; > } > + tdb_unref(tdbp); > return prot; > > drop: > @@ -355,6 +360,7 @@ ipsec_common_input(struct mbuf **mp, int > ipsecstat_inc(ipsec_idrops); > if (tdbp != NULL) > tdbp->tdb_idrops++; > + tdb_unref(tdbp); > return IPPROTO_DONE; > } > > @@ -938,6 +944,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); > } > } > > @@ -945,7 +952,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; > @@ -974,10 +981,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) && > @@ -986,6 +992,7 @@ udpencap_ctlinput(int cmd, struct sockad > ipsec_set_mtu(tdbp, mtu); > } > } > + tdb_unref(first); > } > > void > @@ -1071,6 +1078,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; > } > @@ -1143,6 +1151,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 24 Nov 2021 13:24:23 -0000 > @@ -139,12 +139,16 @@ ipsp_process_packet(struct mbuf *m, stru > */ > if (tdb->tdb_first_use == 0) { > tdb->tdb_first_use = gettime(); > - if (tdb->tdb_flags & TDBF_FIRSTUSE) > - timeout_add_sec(&tdb->tdb_first_tmo, > - tdb->tdb_exp_first_use); > - if (tdb->tdb_flags & TDBF_SOFT_FIRSTUSE) > - timeout_add_sec(&tdb->tdb_sfirst_tmo, > - tdb->tdb_soft_first_use); > + if (tdb->tdb_flags & TDBF_FIRSTUSE) { > + if (timeout_add_sec(&tdb->tdb_first_tmo, > + tdb->tdb_exp_first_use)) > + tdb_ref(tdb); > + } > + if (tdb->tdb_flags & TDBF_SOFT_FIRSTUSE) { > + if (timeout_add_sec(&tdb->tdb_sfirst_tmo, > + tdb->tdb_soft_first_use)) > + tdb_ref(tdb); > + } > } > > /* > @@ -388,6 +392,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 +506,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 +624,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 24 Nov 2021 13:24:23 -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 24 Nov 2021 13:24:23 -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 24 Nov 2021 13:24:23 -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 */ > >