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@

diff --git a/sys/net/if.c b/sys/net/if.c
index 58db412..4534ed5 100644 (file)
--- 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 1141069..8045a81 100644 (file)
--- 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 a3de25d..485928e 100644 (file)
--- 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 1476ac3..e74d623 100644 (file)
--- 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);