From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: ospfd: fix rtlabel refcnt leak in kroute code
To: tech@openbsd.org
Date: Thu, 27 Aug 2026 10:24:13 +0200

The rtlabel handling in kr_change does not properly adjust the refcnt of
the label and so labels may be dropped even though still in use.
On top of this the code does not update the values in case of a change.

I think there are still a few bits missing, e.g. the reload logic seems to
be missing as well. Also the RDE does not track the ext_tag per nexthop so
two routers redistributing the same as-ext LSA with equal metrics but
different ext_tag will use one or the other tag for all routes.

Noticed while working on some imsg cleanup.
-- 
:wq Claudio

Index: kroute.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/kroute.c,v
diff -u -p -r1.118 kroute.c
--- kroute.c	1 Jan 2025 13:44:52 -0000	1.118
+++ kroute.c	27 Aug 2026 08:06:39 -0000
@@ -79,7 +79,7 @@ int	kr_delete_fib(struct kroute_node *);
 
 struct kroute_node	*kroute_find(in_addr_t, u_int8_t, u_int8_t);
 struct kroute_node	*kroute_matchgw(struct kroute_node *, struct in_addr);
-int			 kroute_insert(struct kroute_node *);
+void			 kroute_insert(struct kroute_node *);
 int			 kroute_remove(struct kroute_node *);
 void			 kroute_clear(void);
 
@@ -250,17 +250,30 @@ kr_change_fib(struct kroute_node *kr, st
 					break;
 			}
 
-			if (kn != NULL)
-				/* nexthop already present, skip it */
+			if (kn != NULL) {
+				uint16_t label;
+
+				/* nexthop already present, just change it */
+				kn->r.flags = kroute[i].flags |
+				    F_OSPFD_INSERTED;
+				kn->r.ext_tag = kroute[i].ext_tag;
+				label = kn->r.rtlabel;
+				rtlabel_unref(kn->r.rtlabel);
+				kn->r.rtlabel = rtlabel_tag2id(kn->r.ext_tag);
+				rtlabel_ref(kn->r.rtlabel);
+
+				/* update if label changed */ 
+				if (kn->r.rtlabel != label) {
+					if (send_rtmsg(kr_state.fd, RTM_CHANGE,
+					    &kn->r) == -1)
+					return (-1);
+				}
 				continue;
+			}
 		} else
 			/* modify first entry */
 			kn = kr;
 
-		/* send update */
-		if (send_rtmsg(kr_state.fd, action, &kroute[i]) == -1)
-			return (-1);
-
 		/* create new entry unless we are changing the first entry */
 		if (action == RTM_ADD)
 			if ((kn = calloc(1, sizeof(*kn))) == NULL)
@@ -269,18 +282,21 @@ kr_change_fib(struct kroute_node *kr, st
 		kn->r.prefix.s_addr = kroute[i].prefix.s_addr;
 		kn->r.prefixlen = kroute[i].prefixlen;
 		kn->r.nexthop.s_addr = kroute[i].nexthop.s_addr;
-		kn->r.flags = kroute[i].flags | F_OSPFD_INSERTED;
 		kn->r.priority = kr_state.fib_prio;
+
+		kn->r.flags = kroute[i].flags | F_OSPFD_INSERTED;
 		kn->r.ext_tag = kroute[i].ext_tag;
 		rtlabel_unref(kn->r.rtlabel);	/* for RTM_CHANGE */
-		kn->r.rtlabel = kroute[i].rtlabel;
+		kn->r.rtlabel = rtlabel_tag2id(kn->r.ext_tag);
+		rtlabel_ref(kn->r.rtlabel);
 
 		if (action == RTM_ADD)
-			if (kroute_insert(kn) == -1) {
-				log_debug("kr_update_fib: cannot insert %s",
-				    inet_ntoa(kn->r.nexthop));
-				free(kn);
-			}
+			kroute_insert(kn);
+
+		/* send update */
+		if (send_rtmsg(kr_state.fd, action, &kn->r) == -1)
+			return (-1);
+
 		action = RTM_ADD;
 	}
 	return  (0);
@@ -292,8 +308,6 @@ kr_change(struct kroute *kroute, int krc
 	struct kroute_node	*kr;
 	int			 action = RTM_ADD;
 
-	kroute->rtlabel = rtlabel_tag2id(kroute->ext_tag);
-
 	kr = kroute_find(kroute->prefix.s_addr, kroute->prefixlen,
 	    kr_state.fib_prio);
 	if (kr != NULL && kr->next == NULL && krcount == 1)
@@ -746,7 +760,7 @@ kroute_matchgw(struct kroute_node *kr, s
 	return (NULL);
 }
 
-int
+void
 kroute_insert(struct kroute_node *kr)
 {
 	struct kroute_node	*krm, *krh;
@@ -768,7 +782,7 @@ kroute_insert(struct kroute_node *kr)
 	if (!(kr->r.flags & F_KERNEL)) {
 		/* don't validate or redistribute ospf route */
 		kr->r.flags &= ~F_DOWN;
-		return (0);
+		return;
 	}
 
 	if (kif_validate(kr->r.ifindex))
@@ -777,7 +791,6 @@ kroute_insert(struct kroute_node *kr)
 		kr->r.flags |= F_DOWN;
 
 	kr_redistribute(krh);
-	return (0);
 }
 
 int
Index: name2id.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/name2id.c,v
diff -u -p -r1.2 name2id.c
--- name2id.c	19 Jun 2007 16:45:15 -0000	1.2
+++ name2id.c	27 Aug 2026 07:19:34 -0000
@@ -75,13 +75,11 @@ rtlabel_unref(u_int16_t id)
 	_unref(&rt_labels, id);
 }
 
-/*
 void
 rtlabel_ref(u_int16_t id)
 {
 	_ref(&rt_labels, id);
 }
-*/
 
 u_int16_t
 _name2id(struct n2id_labels *head, const char *name)
Index: ospfd.h
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v
diff -u -p -r1.109 ospfd.h
--- ospfd.h	25 Jun 2026 13:19:06 -0000	1.109
+++ ospfd.h	27 Aug 2026 07:19:53 -0000
@@ -608,6 +608,7 @@ const char	*path_type_name(enum path_typ
 u_int16_t	 rtlabel_name2id(const char *);
 const char	*rtlabel_id2name(u_int16_t);
 void		 rtlabel_unref(u_int16_t);
+void		 rtlabel_ref(u_int16_t);
 u_int32_t	 rtlabel_id2tag(u_int16_t);
 u_int16_t	 rtlabel_tag2id(u_int32_t);
 void		 rtlabel_tag(u_int16_t, u_int32_t);