Fix a problem with how TDB timeouts were used in pfkeyv2.

When we allocated a tdb we did a timeout_add before a timeout_set.
This was a problem in itself, but it shouldn't hurt too much.
What did hurt was that we did a timeout_set after the timeout_add,
timeout_set marked the timeout as not being on the timeout list and if we
did a timeout_del (or timeout_add) later (before the timeout fired) we
ended up with a chunk of freed memory on the timeout queue or maybe
even dangling pointers (or a circular list).

This should probably cure the timeout queue corruption some people were
seeing lately.

1 files changed, 32 insertions, 27 deletions

diff --git a/sys/netinet/ip_ipsp.c b/sys/netinet/ip_ipsp.c
index 135f36048bd..c4b83425334 100644
--- a/sys/netinet/ip_ipsp.c
+++ b/sys/netinet/ip_ipsp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_ipsp.c,v 1.109 2001/03/15 06:30:59 mickey Exp $	*/
+/*	$OpenBSD: ip_ipsp.c,v 1.110 2001/03/27 14:45:22 art Exp $	*/
 
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
@@ -238,8 +238,7 @@ reserve_spi(u_int32_t sspi, u_int32_t tspi, union sockaddr_union *src,
 	if (tdbp != (struct tdb *) NULL)
 	  continue;
 
-	MALLOC(tdbp, struct tdb *, sizeof(struct tdb), M_TDB, M_WAITOK);
-	bzero((caddr_t) tdbp, sizeof(struct tdb));
+	tdbp = tdb_alloc();
 
 	tdbp->tdb_spi = spi;
 	bcopy(&dst->sa, &tdbp->tdb_dst.sa, SA_LEN(&dst->sa));
@@ -247,16 +246,8 @@ reserve_spi(u_int32_t sspi, u_int32_t tspi, union sockaddr_union *src,
 	tdbp->tdb_sproto = sproto;
 	tdbp->tdb_flags |= TDBF_INVALID;       /* Mark SA as invalid for now */
 	tdbp->tdb_satype = SADB_SATYPE_UNSPEC;
-	tdbp->tdb_established = time.tv_sec;
-	tdbp->tdb_epoch = kernfs_epoch - 1;
 	puttdb(tdbp);
 
-	/* Initialize timeouts */
-	timeout_set(&tdbp->tdb_timer_tmo, tdb_timeout, tdbp);
-	timeout_set(&tdbp->tdb_first_tmo, tdb_firstuse, tdbp);
-	timeout_set(&tdbp->tdb_stimer_tmo, tdb_soft_timeout, tdbp);
-	timeout_set(&tdbp->tdb_sfirst_tmo, tdb_soft_firstuse, tdbp);
-
 	/* Setup a "silent" expiration (since TDBF_INVALID's set) */
 	if (ipsec_keep_invalid > 0)
 	{
@@ -749,28 +740,42 @@ tdb_delete(struct tdb *tdbp)
 }
 
 /*
- * Initialize a TDB structure.
+ * Allocate a TDB and initialize a few basic fields.
  */
-int
-tdb_init(struct tdb *tdbp, u_int16_t alg, struct ipsecinit *ii)
+struct tdb *
+tdb_alloc(void)
 {
-    struct xformsw *xsp;
-    int err;
+	struct tdb *tdbp;
 
-    /* Record establishment time */
-    tdbp->tdb_established = time.tv_sec;
-    tdbp->tdb_epoch = kernfs_epoch - 1;
+	MALLOC(tdbp, struct tdb *, sizeof(struct tdb), M_TDB, M_WAITOK);
+	bzero((caddr_t) tdbp, sizeof(struct tdb));
 
-    /* Initialize timeouts */
-    timeout_set(&tdbp->tdb_timer_tmo, tdb_timeout, tdbp);
-    timeout_set(&tdbp->tdb_first_tmo, tdb_firstuse, tdbp);
-    timeout_set(&tdbp->tdb_stimer_tmo, tdb_soft_timeout, tdbp);
-    timeout_set(&tdbp->tdb_sfirst_tmo, tdb_soft_firstuse, tdbp);
+	/* Init Incoming SA-Binding Queues */
+	TAILQ_INIT(&tdbp->tdb_inp);
 
-    /* Init Incoming SA-Binding Queues */
-    TAILQ_INIT(&tdbp->tdb_inp);
+	TAILQ_INIT(&tdbp->tdb_policy_head);
 
-    TAILQ_INIT(&tdbp->tdb_policy_head);
+	/* Record establishment time */
+	tdbp->tdb_established = time.tv_sec;
+	tdbp->tdb_epoch = kernfs_epoch - 1;
+
+	/* Initialize timeouts */
+	timeout_set(&tdbp->tdb_timer_tmo, tdb_timeout, tdbp);
+	timeout_set(&tdbp->tdb_first_tmo, tdb_firstuse, tdbp);
+	timeout_set(&tdbp->tdb_stimer_tmo, tdb_soft_timeout, tdbp);
+	timeout_set(&tdbp->tdb_sfirst_tmo, tdb_soft_firstuse, tdbp);
+
+	return tdbp;
+}
+
+/*
+ * Do further initializations of a TDB.
+ */
+int
+tdb_init(struct tdb *tdbp, u_int16_t alg, struct ipsecinit *ii)
+{
+    struct xformsw *xsp;
+    int err;
 
     for (xsp = xformsw; xsp < xformswNXFORMSW; xsp++)
       if (xsp->xf_type == alg)