Rewrite the transport reference count code to avoid leaks.

hshoexer@ ok.

6 files changed, 84 insertions, 98 deletions

diff --git a/sbin/isakmpd/nat_traversal.c b/sbin/isakmpd/nat_traversal.c
index a358178b0ee..b3fe65e360a 100644
--- a/sbin/isakmpd/nat_traversal.c
+++ b/sbin/isakmpd/nat_traversal.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nat_traversal.c,v 1.5 2004/07/29 08:54:08 ho Exp $	*/
+/*	$OpenBSD: nat_traversal.c,v 1.6 2004/08/03 10:54:09 ho Exp $	*/
 
 /*
  * Copyright (c) 2004 Håkan Olsson.  All rights reserved.
@@ -430,8 +430,12 @@ nat_t_setup_keepalive(struct sa *sa)
 {
 	struct sockaddr *src;
 	struct timeval now;
+	
+	if (sa->initiator)
+		sa->transport->vtbl->get_src(sa->transport, &src);
+	else
+		sa->transport->vtbl->get_dst(sa->transport, &src);
 
-	sa->transport->vtbl->get_src(sa->transport, &src);
 	if (!virtual_listen_lookup(src))
 		return;
 
@@ -443,4 +447,7 @@ nat_t_setup_keepalive(struct sa *sa)
 	if (!sa->nat_t_keepalive)
 		log_print("nat_t_setup_keepalive: "
 		    "timer_add_event() failed, will not send keepalives");
+
+	LOG_DBG((LOG_TRANSPORT, 50, "nat_t_setup_keepalive: "
+	    "added event for phase 1 SA %p", sa));
 }
diff --git a/sbin/isakmpd/transport.c b/sbin/isakmpd/transport.c
index 749468b01bb..a64c374e56c 100644
--- a/sbin/isakmpd/transport.c
+++ b/sbin/isakmpd/transport.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: transport.c,v 1.28 2004/06/21 13:09:00 ho Exp $	 */
+/* $OpenBSD: transport.c,v 1.29 2004/08/03 10:54:09 ho Exp $	 */
 /* $EOM: transport.c,v 1.43 2000/10/10 12:36:39 provos Exp $	 */
 
 /*
@@ -75,17 +75,21 @@ transport_init(void)
 void
 transport_setup(struct transport *t, int toplevel)
 {
-	LOG_DBG((LOG_TRANSPORT, 70, "transport_setup: adding %p%s", t,
-	    toplevel ? " (virtual)" : ""));
-	if (toplevel == 0)
-		LIST_INSERT_HEAD(&transport_list, t, link);
-	else {
-		/* Only the toplevel (virtual) transport has the sendqueues. */
+	if (toplevel) {
+		/* Only the toplevel (virtual) transport has sendqueues.  */
+		LOG_DBG((LOG_TRANSPORT, 70,
+		    "transport_setup: virtual transport %p", t));
 		TAILQ_INIT(&t->sendq);
 		TAILQ_INIT(&t->prio_sendq);
+		t->refcnt = 0;
+	} else {
+		/* udp and udp_encap trp goes into the transport list.  */
+		LOG_DBG((LOG_TRANSPORT, 70,
+		    "transport_setup: added %p to transport list", t));
+		LIST_INSERT_HEAD(&transport_list, t, link);
+		t->refcnt = 1;
 	}
 	t->flags = 0;
-	t->refcnt = 0;
 }
 
 /* Add a referer to transport T.  */
@@ -111,15 +115,6 @@ transport_release(struct transport *t)
 		return;
 
 	LOG_DBG((LOG_TRANSPORT, 70, "transport_release: freeing %p", t));
-	if (t->virtual) {
-		struct virtual_transport *v =
-		    (struct virtual_transport *)t->virtual;
-		if (v->main == t)
-			v->main = 0;
-		else
-			v->encap = 0;
-		LIST_REMOVE(t, link);
-	}
 	t->vtbl->remove(t);
 }
 
@@ -204,7 +199,7 @@ transport_fd_set(fd_set * fds)
 			if (n > max)
 				max = n;
 
-			LOG_DBG((LOG_TRANSPORT,95,"transport_fd_set: "
+			LOG_DBG((LOG_TRANSPORT, 95, "transport_fd_set: "
 			    "transport %p (virtual %p) fd %d", t,
 			    t->virtual, n));
 		}
@@ -228,7 +223,8 @@ transport_pending_wfd_set(fd_set * fds)
 		if (TAILQ_FIRST(&t->virtual->sendq) ||
 		    TAILQ_FIRST(&t->virtual->prio_sendq)) {
 			n = t->vtbl->fd_set(t, fds, 1);
-			LOG_DBG((LOG_TRANSPORT,95,"transport_pending_wfd_set: "
+			LOG_DBG((LOG_TRANSPORT, 95, 
+			    "transport_pending_wfd_set: "
 			    "transport %p (virtual %p) fd %d pending", t,
 			    t->virtual, n));
 			if (n > max)
@@ -278,10 +274,8 @@ transport_send_messages(fd_set * fds)
 	 * Reference all transports first so noone will disappear while in
 	 * use.
 	 */
-	for (t = LIST_FIRST(&transport_list); t; t = LIST_NEXT(t, link)) {
+	for (t = LIST_FIRST(&transport_list); t; t = LIST_NEXT(t, link))
 		transport_reference(t->virtual);
-		transport_reference(t);
-	}
 	
 	for (t = LIST_FIRST(&transport_list); t; t = LIST_NEXT(t, link)) {
 		if ((TAILQ_FIRST(&t->virtual->sendq) ||
@@ -353,6 +347,10 @@ transport_send_messages(fd_set * fds)
 							    "scan");
 					}
 					exchange->last_sent = 0;
+#ifdef notyet
+					exchange_free(exchange);
+					exchange = 0;
+#endif
 				} else {
 					gettimeofday(&expiration, 0);
 
@@ -412,7 +410,6 @@ transport_send_messages(fd_set * fds)
 	for (t = LIST_FIRST(&transport_list); t; t = next) {
 		next = LIST_NEXT(t, link);
 		transport_release(t->virtual);
-		transport_release(t);
 	}
 }
 
diff --git a/sbin/isakmpd/udp.c b/sbin/isakmpd/udp.c
index 20c2ce42a88..65b88e92c5e 100644
--- a/sbin/isakmpd/udp.c
+++ b/sbin/isakmpd/udp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: udp.c,v 1.77 2004/06/25 19:42:38 mcbride Exp $	 */
+/* $OpenBSD: udp.c,v 1.78 2004/08/03 10:54:09 ho Exp $	 */
 /* $EOM: udp.c,v 1.57 2001/01/26 10:09:57 niklas Exp $	 */
 
 /*
@@ -76,9 +76,9 @@ int		  udp_fd_isset(struct transport *, fd_set *);
 void		  udp_get_dst(struct transport *, struct sockaddr **);
 void		  udp_get_src(struct transport *, struct sockaddr **);
 char		 *udp_decode_ids(struct transport *);
+void		  udp_remove(struct transport *);
 
 static struct transport *udp_create(char *);
-static void     udp_remove(struct transport *);
 static void     udp_report(struct transport *);
 static void     udp_handle_message(struct transport *);
 static struct transport *udp_make(struct sockaddr *);
@@ -193,7 +193,6 @@ udp_make(struct sockaddr *laddr)
 		free (tstr);
 	}
 	transport_setup(&t->transport, 0);
-	transport_reference(&t->transport);
 	t->transport.flags |= TRANSPORT_LISTEN;
 	return &t->transport;
 
@@ -355,10 +354,8 @@ udp_create(char *name)
 	}
 	t = (struct transport *)v;
 	rv = udp_clone(v->main, dst);
-	if (rv) {
+	if (rv)
 		rv->vtbl = &udp_transport_vtbl;
-		transport_reference(rv->virtual);
-	}
 
 ret:
 	if (addr_list)
@@ -376,12 +373,12 @@ udp_remove(struct transport *t)
 		free(u->src);
 	if (u->dst)
 		free(u->dst);
-	if (t->flags & TRANSPORT_LISTEN) {
-		if (u->s >= 0)
-			close(u->s);
-		if (u->link.le_prev)
-			LIST_REMOVE(u, link);
-	}
+	if ((t->flags & TRANSPORT_LISTEN) && u->s >= 0)
+		close(u->s);
+	if (t->link.le_prev)
+		LIST_REMOVE(t, link);
+
+	LOG_DBG((LOG_TRANSPORT, 90, "udp_remove: removed transport %p", t));
 	free(t);
 }
 
diff --git a/sbin/isakmpd/udp.h b/sbin/isakmpd/udp.h
index 44916ccc044..dd91a6ce7e4 100644
--- a/sbin/isakmpd/udp.h
+++ b/sbin/isakmpd/udp.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: udp.h,v 1.9 2004/06/20 15:24:05 ho Exp $	 */
+/* $OpenBSD: udp.h,v 1.10 2004/08/03 10:54:09 ho Exp $	 */
 /* $EOM: udp.h,v 1.4 1998/12/22 02:23:43 niklas Exp $	 */
 
 /*
@@ -47,7 +47,6 @@ struct udp_transport {
 	struct sockaddr	 *src;
 	struct sockaddr	 *dst;
 	int		  s;
-	LIST_ENTRY(udp_transport) link;
 };
 
 #endif				/* _UDP_H_ */
diff --git a/sbin/isakmpd/udp_encap.c b/sbin/isakmpd/udp_encap.c
index 0ad696d950c..c59bd0c7636 100644
--- a/sbin/isakmpd/udp_encap.c
+++ b/sbin/isakmpd/udp_encap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: udp_encap.c,v 1.4 2004/07/07 09:13:01 hshoexer Exp $	*/
+/*	$OpenBSD: udp_encap.c,v 1.5 2004/08/03 10:54:09 ho Exp $	*/
 
 /*
  * Copyright (c) 1998, 1999, 2001 Niklas Hallqvist.  All rights reserved.
@@ -67,26 +67,26 @@
 #endif
 
 /* Reused, from udp.c */
-struct transport *udp_clone (struct transport *, struct sockaddr *);
+struct transport *udp_clone(struct transport *, struct sockaddr *);
 int		  udp_fd_set(struct transport *, fd_set *, int);
 int		  udp_fd_isset(struct transport *, fd_set *);
-void		  udp_get_dst (struct transport *, struct sockaddr **);
-void		  udp_get_src (struct transport *, struct sockaddr **);
-char		 *udp_decode_ids (struct transport *);
-
-static struct transport *udp_encap_create (char *);
-static void		 udp_encap_remove (struct transport *);
-static void		 udp_encap_report (struct transport *);
-static void		 udp_encap_handle_message (struct transport *);
-static struct transport *udp_encap_make (struct sockaddr *);
-static int		 udp_encap_send_message (struct message *,
+void		  udp_get_dst(struct transport *, struct sockaddr **);
+void		  udp_get_src(struct transport *, struct sockaddr **);
+char		 *udp_decode_ids(struct transport *);
+void		  udp_remove(struct transport *);
+
+static struct transport *udp_encap_create(char *);
+static void		 udp_encap_report(struct transport *);
+static void		 udp_encap_handle_message(struct transport *);
+static struct transport *udp_encap_make(struct sockaddr *);
+static int		 udp_encap_send_message(struct message *,
     struct transport *);
 
 static struct transport_vtbl udp_encap_transport_vtbl = {
 	{ 0 }, "udp_encap",
 	udp_encap_create,
 	0,
-	udp_encap_remove,
+	udp_remove,
 	udp_encap_report,
 	udp_fd_set,
 	udp_fd_isset,
@@ -191,7 +191,6 @@ udp_encap_make(struct sockaddr *laddr)
 		free(tstr);
 	}
 	transport_setup(&t->transport, 0);
-	transport_reference(&t->transport);
 	t->transport.flags |= TRANSPORT_LISTEN;
 	return &t->transport;
 
@@ -201,7 +200,7 @@ err:
 	if (t) {
 		/* Already closed.  */
 		t->s = -1;
-		udp_encap_remove(&t->transport);
+		udp_remove(&t->transport);
 	}
 	return 0;
 }
@@ -312,36 +311,16 @@ udp_encap_create(char *name)
 	}
 	t = (struct transport *)v;
 	rv = udp_clone(v->encap, dst);
-	if (rv) {
-		rv->vtbl = &udp_encap_transport_vtbl; /* XXX Necessary? */
-		transport_reference(rv->virtual);
-	}
-  
-  ret:
+	if (rv)
+		rv->vtbl = &udp_encap_transport_vtbl;
+
+ret:
 	if (addr_list)
 		conf_free_list(addr_list);
 	free(dst);
 	return rv;
 }
 
-void
-udp_encap_remove(struct transport *t)
-{
-	struct udp_transport *u = (struct udp_transport *)t;
-
-	if (u->src)
-		free (u->src);
-	if (u->dst)
-		free (u->dst);
-	if (t->flags & TRANSPORT_LISTEN) {
-		if (u->s >= 0)
-			close (u->s);
-		if (u->link.le_prev)
-			LIST_REMOVE (u, link);
-	}
-	free (t);
-}
-
 /* Report transport-method specifics of the T transport.  */
 void
 udp_encap_report(struct transport *t)
diff --git a/sbin/isakmpd/virtual.c b/sbin/isakmpd/virtual.c
index 9f5fdfc9774..bd0dcb92415 100644
--- a/sbin/isakmpd/virtual.c
+++ b/sbin/isakmpd/virtual.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: virtual.c,v 1.5 2004/07/08 19:53:46 hshoexer Exp $	*/
+/*	$OpenBSD: virtual.c,v 1.6 2004/08/03 10:54:09 ho Exp $	*/
 
 /*
  * Copyright (c) 2004 Håkan Olsson.  All rights reserved.
@@ -126,6 +126,7 @@ virtual_init(void)
 			return;
 		LIST_INSERT_HEAD(&virtual_listen_list,
 		    (struct virtual_transport *)default_transport, link);
+		transport_reference(default_transport);
 	}
 
 	if (!bind_family || (bind_family & BIND_FAMILY_INET6)) {
@@ -134,7 +135,10 @@ virtual_init(void)
 			return;
 		LIST_INSERT_HEAD(&virtual_listen_list,
 		    (struct virtual_transport *)default_transport6, link);
+		transport_reference(default_transport6);
 	}
+
+	return;
 }
 
 struct virtual_transport *
@@ -194,11 +198,18 @@ struct virtual_transport *
 virtual_listen_lookup(struct sockaddr *addr)
 {
 	struct virtual_transport *v;
-	struct udp_transport	*u;
+	struct udp_transport	 *u;
 
 	for (v = LIST_FIRST(&virtual_listen_list); v;
 	     v = LIST_NEXT(v, link)) {
-		u = (struct udp_transport *)v->main;
+		if (!(u = (struct udp_transport *)v->main))
+			if (!(u = (struct udp_transport *)v->encap)) {
+				log_print("virtual_listen_lookup: "
+				    "virtual %p has no low-level transports",
+				    v);
+				continue;
+			}
+
 		if (u->src->sa_family == addr->sa_family
 		    && sockaddr_addrlen(u->src) == sockaddr_addrlen(addr)
 		    && memcmp(sockaddr_addrdata (u->src),
@@ -207,6 +218,7 @@ virtual_listen_lookup(struct sockaddr *addr)
 			return v;
 	}
 
+	LOG_DBG((LOG_TRANSPORT, 40, "virtual_listen_lookup: no match"));
 	return 0;
 }
 
@@ -267,7 +279,7 @@ virtual_bind(const struct sockaddr *addr)
 	if (*ep != '\0' || lport < 0 || lport > USHRT_MAX) {
 		log_print("virtual_bind: "
 		    "port string \"%s\" not convertible to in_port_t", port);
-		transport_release(v->main);
+		v->main->vtbl->remove(v->main);
 		free(v);
 		return 0;
 	}
@@ -275,7 +287,7 @@ virtual_bind(const struct sockaddr *addr)
 	sockaddr_set_port((struct sockaddr *)&tmp_sa, (in_port_t)lport);
 	v->encap = udp_encap_bind((struct sockaddr *)&tmp_sa);
 	if (!v->encap) {
-		transport_release(v->main);
+		v->main->vtbl->remove(v->main);
 		free(v);
 		return 0;
 	}
@@ -284,7 +296,6 @@ virtual_bind(const struct sockaddr *addr)
 	v->encap_is_active = 0;
 
 	transport_setup(&v->transport, 1);
-	transport_reference(&v->transport);
 	v->transport.flags |= TRANSPORT_LISTEN;
 
 	return (struct transport *)v;
@@ -484,6 +495,7 @@ virtual_bind_if(char *ifname, struct sockaddr *if_addr, void *arg)
 	}
 	LIST_INSERT_HEAD(&virtual_listen_list, (struct virtual_transport *)t,
 	    link);
+	transport_reference(t);
 	return 0;
 }
 
@@ -503,6 +515,9 @@ virtual_clone(struct transport *vt, struct sockaddr *raddr)
 	v2 = (struct virtual_transport *)t;
 
 	memcpy(v2, v, sizeof *v);
+	/* Remove the copy's links into virtual_listen_list.  */
+	v2->link.le_next = 0;
+	v2->link.le_prev = 0;
 
 	if (v->encap_is_active)
 		v2->main = 0; /* No need to clone this.  */
@@ -523,13 +538,6 @@ virtual_clone(struct transport *vt, struct sockaddr *raddr)
 
 	t->flags &= ~TRANSPORT_LISTEN;
 	transport_setup(t, 1);
-
-	transport_reference(t);
-	if (v2->main)
-		transport_reference(v2->main);
-	if (v2->encap)
-		transport_reference(v2->encap);
-
 	return t;
 }
   
@@ -572,8 +580,6 @@ virtual_create(char *name)
 	if (t2)
 		t2->virtual = (struct transport *)v;
 	transport_setup(&v->transport, 1);
-	LIST_INSERT_HEAD(&virtual_listen_list, v, link);
-
 	return (struct transport *)v;
 }
 
@@ -589,6 +595,7 @@ virtual_remove(struct transport *t)
 	if (v->link.le_prev)
 		LIST_REMOVE(v, link);
 
+	LOG_DBG((LOG_TRANSPORT, 90, "virtual_remove: removed %p", v));
 	free(t);
 }
 
@@ -638,7 +645,7 @@ virtual_send_message(struct message *msg, struct transport *t)
 	struct virtual_transport *v =
 	    (struct virtual_transport *)msg->transport;
 #if defined (USE_NAT_TRAVERSAL)
-	struct sockaddr *sa;
+	struct sockaddr *dst;
 	in_port_t port;
 	
 	/*
@@ -657,11 +664,11 @@ virtual_send_message(struct message *msg, struct transport *t)
 		v->encap_is_active++;
 
 		/* Copy destination port if it is translated (NAT).  */
-		v->main->vtbl->get_dst(v->main, &sa);
-		port = ntohs(sockaddr_port(sa));
+		v->main->vtbl->get_dst(v->main, &dst);
+		port = ntohs(sockaddr_port(dst));
 		if (port != UDP_DEFAULT_PORT) {
-			v->main->vtbl->get_dst(v->encap, &sa);
-			sockaddr_set_port(sa, port);
+			v->main->vtbl->get_dst(v->encap, &dst);
+			sockaddr_set_port(dst, port);
 		}
 	}
 #endif /* USE_NAT_TRAVERSAL */