Introduce `rtlabel_mtx' mutex(9) to protect route labels storage. This

time kernel and net locks are held in various combination to protect it.
We don't want to put kernel lock to all the places. Netlock also can't
be used  because rtfree(9) which calls rtlabel_unref() has unknown
netlock state within.

This new `rtlabel_mtx' mutex(9) protects `rt_labels' list and `label'
entry dereference. Since we don't export 'rt_label' structure, keep this
lock private to net/route.c. For this reason rtlabel_id2name() now
copies label string to externally passed buffer instead of returning
address of `rt_labels' list data. This is the way which rtlabel_id2sa()
already works.

ok bluhm@

4 files changed, 71 insertions, 31 deletions

diff --git a/sys/net/if.c b/sys/net/if.c
index 58db412810d..4534ed54faa 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if.c,v 1.693 2023/04/26 00:14:21 jan Exp $	*/
+/*	$OpenBSD: if.c,v 1.694 2023/04/26 19:54:35 mvs Exp $	*/
 /*	$NetBSD: if.c,v 1.35 1996/05/07 05:26:04 thorpej Exp $	*/
 
 /*
@@ -2388,7 +2388,6 @@ ifioctl_get(u_long cmd, caddr_t data)
 	char ifrtlabelbuf[RTLABEL_LEN];
 	int error = 0;
 	size_t bytesdone;
-	const char *label;
 
 	switch(cmd) {
 	case SIOCGIFCONF:
@@ -2463,9 +2462,8 @@ ifioctl_get(u_long cmd, caddr_t data)
 		break;
 
 	case SIOCGIFRTLABEL:
-		if (ifp->if_rtlabelid &&
-		    (label = rtlabel_id2name(ifp->if_rtlabelid)) != NULL) {
-			strlcpy(ifrtlabelbuf, label, RTLABEL_LEN);
+		if (ifp->if_rtlabelid && rtlabel_id2name(ifp->if_rtlabelid,
+		    ifrtlabelbuf, RTLABEL_LEN) != NULL) {
 			error = copyoutstr(ifrtlabelbuf, ifr->ifr_data,
 			    RTLABEL_LEN, &bytesdone);
 		} else
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index 1141069dcf6..8045a818d03 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_ioctl.c,v 1.397 2023/01/06 17:44:34 sashan Exp $ */
+/*	$OpenBSD: pf_ioctl.c,v 1.398 2023/04/26 19:54:35 mvs Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -491,15 +491,11 @@ pf_rtlabel_remove(struct pf_addr_wrap *a)
 void
 pf_rtlabel_copyout(struct pf_addr_wrap *a)
 {
-	const char	*name;
-
 	if (a->type == PF_ADDR_RTLABEL && a->v.rtlabel) {
-		if ((name = rtlabel_id2name(a->v.rtlabel)) == NULL)
+		if (rtlabel_id2name(a->v.rtlabel, a->v.rtlabelname,
+		    sizeof(a->v.rtlabelname)) == NULL)
 			strlcpy(a->v.rtlabelname, "?",
 			    sizeof(a->v.rtlabelname));
-		else
-			strlcpy(a->v.rtlabelname, name,
-			    sizeof(a->v.rtlabelname));
 	}
 }
 
diff --git a/sys/net/route.c b/sys/net/route.c
index a3de25d7b24..485928ef618 100644
--- a/sys/net/route.c
+++ b/sys/net/route.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: route.c,v 1.418 2023/04/26 16:09:44 kn Exp $	*/
+/*	$OpenBSD: route.c,v 1.419 2023/04/26 19:54:35 mvs Exp $	*/
 /*	$NetBSD: route.c,v 1.14 1996/02/13 22:00:46 christos Exp $	*/
 
 /*
@@ -113,6 +113,7 @@
 #include <sys/queue.h>
 #include <sys/pool.h>
 #include <sys/atomic.h>
+#include <sys/mutex.h>
 
 #include <net/if.h>
 #include <net/if_var.h>
@@ -137,6 +138,12 @@
 #include <net/bfd.h>
 #endif
 
+/*
+ * Locks used to protect struct members:
+ *      I       immutable after creation
+ *      L       rtlabel_mtx
+ */
+
 #define ROUNDUP(a) (a>0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) : sizeof(long))
 
 /* Give some jitter to hash, to avoid synchronization between routers. */
@@ -163,13 +170,15 @@ static int rt_copysa(struct sockaddr *, struct sockaddr *, struct sockaddr **);
 #define	LABELID_MAX	50000
 
 struct rt_label {
-	TAILQ_ENTRY(rt_label)	rtl_entry;
-	char			rtl_name[RTLABEL_LEN];
-	u_int16_t		rtl_id;
-	int			rtl_ref;
+	TAILQ_ENTRY(rt_label)	rtl_entry;		/* [L] */
+	char			rtl_name[RTLABEL_LEN];	/* [I] */
+	u_int16_t		rtl_id;			/* [I] */
+	int			rtl_ref;		/* [L] */
 };
 
-TAILQ_HEAD(rt_labels, rt_label)	rt_labels = TAILQ_HEAD_INITIALIZER(rt_labels);
+TAILQ_HEAD(rt_labels, rt_label)	rt_labels =
+    TAILQ_HEAD_INITIALIZER(rt_labels);		/* [L] */
+struct mutex rtlabel_mtx = MUTEX_INITIALIZER(IPL_NET);
 
 void
 route_init(void)
@@ -1596,15 +1605,17 @@ u_int16_t
 rtlabel_name2id(char *name)
 {
 	struct rt_label		*label, *p;
-	u_int16_t		 new_id = 1;
+	u_int16_t		 new_id = 1, id = 0;
 
 	if (!name[0])
 		return (0);
 
+	mtx_enter(&rtlabel_mtx);
 	TAILQ_FOREACH(label, &rt_labels, rtl_entry)
 		if (strcmp(name, label->rtl_name) == 0) {
 			label->rtl_ref++;
-			return (label->rtl_id);
+			id = label->rtl_id;
+			goto out;
 		}
 
 	/*
@@ -1618,11 +1629,11 @@ rtlabel_name2id(char *name)
 		new_id = p->rtl_id + 1;
 	}
 	if (new_id > LABELID_MAX)
-		return (0);
+		goto out;
 
 	label = malloc(sizeof(*label), M_RTABLE, M_NOWAIT|M_ZERO);
 	if (label == NULL)
-		return (0);
+		goto out;
 	strlcpy(label->rtl_name, name, sizeof(label->rtl_name));
 	label->rtl_id = new_id;
 	label->rtl_ref++;
@@ -1632,14 +1643,20 @@ rtlabel_name2id(char *name)
 	else		/* either list empty or no free slot in between */
 		TAILQ_INSERT_TAIL(&rt_labels, label, rtl_entry);
 
-	return (label->rtl_id);
+	id = label->rtl_id;
+out:
+	mtx_leave(&rtlabel_mtx);
+
+	return (id);
 }
 
 const char *
-rtlabel_id2name(u_int16_t id)
+rtlabel_id2name_locked(u_int16_t id)
 {
 	struct rt_label	*label;
 
+	MUTEX_ASSERT_LOCKED(&rtlabel_mtx);
+
 	TAILQ_FOREACH(label, &rt_labels, rtl_entry)
 		if (label->rtl_id == id)
 			return (label->rtl_name);
@@ -1647,18 +1664,44 @@ rtlabel_id2name(u_int16_t id)
 	return (NULL);
 }
 
+const char *
+rtlabel_id2name(u_int16_t id, char *rtlabelbuf, size_t sz)
+{
+	const char *label;
+
+	if (id == 0)
+		return (NULL);
+
+	mtx_enter(&rtlabel_mtx);
+	if ((label = rtlabel_id2name_locked(id)) != NULL)
+		strlcpy(rtlabelbuf, label, sz);
+	mtx_leave(&rtlabel_mtx);
+
+	if (label == NULL)
+		return (NULL);
+
+	return (rtlabelbuf);
+}
+
 struct sockaddr *
 rtlabel_id2sa(u_int16_t labelid, struct sockaddr_rtlabel *sa_rl)
 {
 	const char	*label;
 
-	if (labelid == 0 || (label = rtlabel_id2name(labelid)) == NULL)
+	if (labelid == 0)
 		return (NULL);
 
-	bzero(sa_rl, sizeof(*sa_rl));
-	sa_rl->sr_len = sizeof(*sa_rl);
-	sa_rl->sr_family = AF_UNSPEC;
-	strlcpy(sa_rl->sr_label, label, sizeof(sa_rl->sr_label));
+	mtx_enter(&rtlabel_mtx);
+	if ((label = rtlabel_id2name_locked(labelid)) != NULL) {
+		bzero(sa_rl, sizeof(*sa_rl));
+		sa_rl->sr_len = sizeof(*sa_rl);
+		sa_rl->sr_family = AF_UNSPEC;
+		strlcpy(sa_rl->sr_label, label, sizeof(sa_rl->sr_label));
+	}
+	mtx_leave(&rtlabel_mtx);
+
+	if (label == NULL)
+		return (NULL);
 
 	return ((struct sockaddr *)sa_rl);
 }
@@ -1671,6 +1714,7 @@ rtlabel_unref(u_int16_t id)
 	if (id == 0)
 		return;
 
+	mtx_enter(&rtlabel_mtx);
 	TAILQ_FOREACH_SAFE(p, &rt_labels, rtl_entry, next) {
 		if (id == p->rtl_id) {
 			if (--p->rtl_ref == 0) {
@@ -1680,6 +1724,7 @@ rtlabel_unref(u_int16_t id)
 			break;
 		}
 	}
+	mtx_leave(&rtlabel_mtx);
 }
 
 int
diff --git a/sys/net/route.h b/sys/net/route.h
index 1476ac35956..e74d623fc0b 100644
--- a/sys/net/route.h
+++ b/sys/net/route.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: route.h,v 1.198 2023/01/28 10:17:16 mvs Exp $	*/
+/*	$OpenBSD: route.h,v 1.199 2023/04/26 19:54:35 mvs Exp $	*/
 /*	$NetBSD: route.h,v 1.9 1996/02/13 22:00:49 christos Exp $	*/
 
 /*
@@ -416,7 +416,8 @@ struct rttimer_queue {
 	int				rtq_timeout;	/* [T] */
 };
 
-const char	*rtlabel_id2name(u_int16_t);
+const char	*rtlabel_id2name_locked(u_int16_t);
+const char	*rtlabel_id2name(u_int16_t, char *, size_t);
 u_int16_t	 rtlabel_name2id(char *);
 struct sockaddr	*rtlabel_id2sa(u_int16_t, struct sockaddr_rtlabel *);
 void		 rtlabel_unref(u_int16_t);