Plug some memory leaks in rde. Based on a patch by Patrick Latifi.

Added attr_move() so that we can copy the attribute before calling the filter.
path_update() will now use the passed attribute so it can't be simply reused.
OK henning@

4 files changed, 38 insertions, 14 deletions

diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c
index b939f9452e5..fabeb4f7700 100644
--- a/usr.sbin/bgpd/rde.c
+++ b/usr.sbin/bgpd/rde.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.c,v 1.93 2004/03/02 19:29:01 claudio Exp $ */
+/*	$OpenBSD: rde.c,v 1.94 2004/03/05 22:21:32 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -388,7 +388,7 @@ rde_update_dispatch(struct imsg *imsg)
 	u_int16_t		 nlri_len, size;
 	u_int8_t		 prefixlen, subtype;
 	struct bgpd_addr	 prefix;
-	struct attr_flags	 attrs;
+	struct attr_flags	 attrs, fattrs;
 
 	peer = peer_get(imsg->hdr.peerid);
 	if (peer == NULL)	/* unknown peer, cannot happen */
@@ -509,14 +509,17 @@ rde_update_dispatch(struct imsg *imsg)
 		p += pos;
 		nlri_len -= pos;
 
-		/* input filter */
 		/*
-		 * XXX we need to copy attrs befor calling the filter
-		 * but that stinks, because we copy it again in path_update.
+		 * We need to copy attrs befor calling the filter because
+		 * the filter may change the attributes.
 		 */
-		if (rde_filter(peer, &attrs, &prefix, prefixlen,
-		    DIR_IN) == ACTION_DENY)
+		attr_copy(&fattrs, &attrs);
+		/* input filter */
+		if (rde_filter(peer, &fattrs, &prefix, prefixlen,
+		    DIR_IN) == ACTION_DENY) {
+			attr_free(&fattrs);
 			continue;
+		}
 
 		/* max prefix checker */
 		if (peer->conf.max_prefix &&
@@ -525,11 +528,12 @@ rde_update_dispatch(struct imsg *imsg)
 			rde_update_err(peer, ERR_CEASE, ERR_CEASE_MAX_PREFIX,
 			    NULL, 0);
 			attr_free(&attrs);
+			attr_free(&fattrs);
 			return (-1);
 		}
 
-		rde_update_log("update", peer, &attrs, &prefix, prefixlen);
-		path_update(peer, &attrs, &prefix, prefixlen);
+		rde_update_log("update", peer, &fattrs, &prefix, prefixlen);
+		path_update(peer, &fattrs, &prefix, prefixlen);
 	}
 
 	/* need to free allocated attribute memory that is no longer used */
diff --git a/usr.sbin/bgpd/rde.h b/usr.sbin/bgpd/rde.h
index 7ba91ef7f14..1822e578e34 100644
--- a/usr.sbin/bgpd/rde.h
+++ b/usr.sbin/bgpd/rde.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.h,v 1.33 2004/03/01 16:02:01 claudio Exp $ */
+/*	$OpenBSD: rde.h,v 1.34 2004/03/05 22:21:32 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org> and
@@ -226,6 +226,7 @@ u_char		*attr_error(u_char *, u_int16_t, struct attr_flags *,
 u_int8_t	 attr_missing(struct attr_flags *, int);
 int		 attr_compare(struct attr_flags *, struct attr_flags *);
 void		 attr_copy(struct attr_flags *, struct attr_flags *);
+void		 attr_move(struct attr_flags *, struct attr_flags *);
 void		 attr_free(struct attr_flags *);
 int		 attr_write(void *, u_int16_t, u_int8_t, u_int8_t, void *,
 		     u_int16_t);
diff --git a/usr.sbin/bgpd/rde_attr.c b/usr.sbin/bgpd/rde_attr.c
index 2705b668138..3b8e6f609f4 100644
--- a/usr.sbin/bgpd/rde_attr.c
+++ b/usr.sbin/bgpd/rde_attr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde_attr.c,v 1.17 2004/03/01 17:04:07 henning Exp $ */
+/*	$OpenBSD: rde_attr.c,v 1.18 2004/03/05 22:21:32 claudio Exp $ */
 
 /*
  * Copyright (c) 2004 Claudio Jeker <claudio@openbsd.org>
@@ -393,6 +393,21 @@ attr_copy(struct attr_flags *t, struct attr_flags *s)
 }
 
 void
+attr_move(struct attr_flags *t, struct attr_flags *s)
+{
+	struct attr	*os;
+	/*
+	 * first copy the full struct, then move the optional attributes.
+	 */
+	memcpy(t, s, sizeof(struct attr_flags));
+	TAILQ_INIT(&t->others);
+	while ((os = TAILQ_FIRST(&s->others)) != NULL) {
+		TAILQ_REMOVE(&s->others, os, attr_l);
+		TAILQ_INSERT_TAIL(&t->others, os, attr_l);
+	}
+}
+
+void
 attr_free(struct attr_flags *a)
 {
 	/*
@@ -467,9 +482,12 @@ attr_optadd(struct attr_flags *attr, u_int8_t flags, u_int8_t type,
 
 	/* keep a sorted list */
 	TAILQ_FOREACH_REVERSE(p, &attr->others, attr_l, attr_list) {
-		if (type == p->type)
+		if (type == p->type) {
 			/* attribute only once allowed */
+			free(a->data);
+			free(a);
 			return (-1);
+		}
 		if (type > p->type) {
 			TAILQ_INSERT_AFTER(&attr->others, p, a, attr_l);
 			return (0);
diff --git a/usr.sbin/bgpd/rde_rib.c b/usr.sbin/bgpd/rde_rib.c
index d90dcb63590..9195432e080 100644
--- a/usr.sbin/bgpd/rde_rib.c
+++ b/usr.sbin/bgpd/rde_rib.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde_rib.c,v 1.40 2004/03/01 16:02:01 claudio Exp $ */
+/*	$OpenBSD: rde_rib.c,v 1.41 2004/03/05 22:21:32 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org>
@@ -117,6 +117,7 @@ path_update(struct rde_peer *peer, struct attr_flags *attrs,
 		if (attr_compare(&asp->flags, attrs) == 0) {
 			/* path are equal, just add prefix */
 			pte = prefix_add(asp, prefix, prefixlen);
+			attr_free(attrs);
 		} else {
 			/* non equal path attributes create new path */
 			if ((p = prefix_get(asp, prefix, prefixlen)) == NULL) {
@@ -159,7 +160,7 @@ path_add(struct rde_peer *peer, struct attr_flags *attr)
 
 	asp = path_alloc();
 
-	attr_copy(&asp->flags, attr);
+	attr_move(&asp->flags, attr);
 
 	path_link(asp, peer);
 	return asp;