... and use it to protect route labels storage. This time it is not
clean, which lock protects it because we holding kernel and net locks in
various combinations while accessing it. I see no reason to put kernel
lock to all the places. Also netlock could not be used, because rtfree()
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, I want to
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?
Index: sys/net/if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.692
diff -u -p -r1.692 if.c
--- sys/net/if.c 22 Apr 2023 04:39:46 -0000 1.692
+++ sys/net/if.c 24 Apr 2023 15:11:33 -0000
@@ -2383,7 +2383,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:
@@ -2458,9 +2457,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
Index: sys/net/pf_ioctl.c
===================================================================
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.397
diff -u -p -r1.397 pf_ioctl.c
--- sys/net/pf_ioctl.c 6 Jan 2023 17:44:34 -0000 1.397
+++ sys/net/pf_ioctl.c 24 Apr 2023 15:11:33 -0000
@@ -491,14 +491,10 @@ 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));
}
}
Index: sys/net/route.c
===================================================================
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.416
diff -u -p -r1.416 route.c
--- sys/net/route.c 28 Jan 2023 10:17:16 -0000 1.416
+++ sys/net/route.c 24 Apr 2023 15:11:33 -0000
@@ -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 *,
#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)
@@ -1603,15 +1612,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;
}
/*
@@ -1625,11 +1636,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++;
@@ -1639,14 +1650,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);
@@ -1654,18 +1671,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);
}
@@ -1678,6 +1721,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) {
@@ -1687,6 +1731,7 @@ rtlabel_unref(u_int16_t id)
break;
}
}
+ mtx_leave(&rtlabel_mtx);
}
int
Index: sys/net/route.h
===================================================================
RCS file: /cvs/src/sys/net/route.h,v
retrieving revision 1.198
diff -u -p -r1.198 route.h
--- sys/net/route.h 28 Jan 2023 10:17:16 -0000 1.198
+++ sys/net/route.h 24 Apr 2023 15:11:33 -0000
@@ -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);
