> On 7 Dec 2021, at 18:31, Alexander Bluhm <[email protected]> wrote:
>
> Hi,
>
> In ipo_tdb the flow contains a reference counted TDB cache. This
> may prevent that tdb_free() is called. It is not a real leak as
> ipsecctl -F or termination of iked flush this cache. The kernel
> does the cleanup itself if we move the code from tdb_free() to
> tdb_delete().
>
> ok?
>
With your early TDB refcounting diff we it was exposed we had TDB
linked to it's `tdb_policy_head’.
ok mvs@
> bluhm
>
> Index: netinet/ip_ipsp.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.c,v
> retrieving revision 1.261
> diff -u -p -r1.261 ip_ipsp.c
> --- netinet/ip_ipsp.c 3 Dec 2021 19:04:49 -0000 1.261
> +++ netinet/ip_ipsp.c 7 Dec 2021 15:27:16 -0000
> @@ -923,6 +923,19 @@ tdb_unlink_locked(struct tdb *tdbp)
> }
>
> void
> +tdb_cleanspd(struct tdb *tdbp)
> +{
> + struct ipsec_policy *ipo;
> +
> + 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. */
> + }
> +}
> +
> +void
> tdb_unbundle(struct tdb *tdbp)
> {
> if (tdbp->tdb_onext != NULL) {
> @@ -1001,6 +1014,8 @@ tdb_dodelete(struct tdb *tdbp, int locke
> else
> tdb_unlink(tdbp);
>
> + /* cleanup SPD references */
> + tdb_cleanspd(tdbp);
> /* release tdb_onext/tdb_inext references */
> tdb_unbundle(tdbp);
> /* delete timeouts and release references */
> @@ -1043,8 +1058,6 @@ tdb_alloc(u_int rdomain)
> void
> tdb_free(struct tdb *tdbp)
> {
> - struct ipsec_policy *ipo;
> -
> NET_ASSERT_LOCKED();
>
> if (tdbp->tdb_xform) {
> @@ -1057,13 +1070,7 @@ tdb_free(struct tdb *tdbp)
> pfsync_delete_tdb(tdbp);
> #endif
>
> - /* Cleanup SPD references. */
> - 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. */
> - }
> + KASSERT(TAILQ_EMPTY(&tdbp->tdb_policy_head));
>
> if (tdbp->tdb_ids) {
> ipsp_ids_free(tdbp->tdb_ids);
> Index: netinet/ip_ipsp.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.h,v
> retrieving revision 1.227
> diff -u -p -r1.227 ip_ipsp.h
> --- netinet/ip_ipsp.h 3 Dec 2021 19:04:49 -0000 1.227
> +++ netinet/ip_ipsp.h 7 Dec 2021 13:48:51 -0000
> @@ -577,6 +577,7 @@ 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_cleanspd(struct tdb *);
> void tdb_unbundle(struct tdb *);
> void tdb_deltimeouts(struct tdb *);
> int tdb_walk(u_int, int (*)(struct tdb *, void *, int), void *);
>
