The kroute code can leak rtlabel references in various conditions.
In the end we want to drop the reference before free().
So move the unref into kroute_remove() so it is done mostly in one place.
On top fix a few other places where kroutes are freed.
This diff is on top of the previous diff (removal of F_RTLABEL).
--
:wq Claudio
Index: kroute.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
retrieving revision 1.248
diff -u -p -r1.248 kroute.c
--- kroute.c 5 Jun 2022 12:43:13 -0000 1.248
+++ kroute.c 7 Jun 2022 15:43:42 -0000
@@ -507,6 +507,7 @@ kr4_change(struct ktable *kt, struct kro
kr->r.labelid = labelid;
if (kroute_insert(kt, kr) == -1) {
+ rtlabel_unref(kr->r.labelid);
free(kr);
return (-1);
}
@@ -565,6 +566,7 @@ kr6_change(struct ktable *kt, struct kro
kr6->r.labelid = labelid;
if (kroute6_insert(kt, kr6) == -1) {
+ rtlabel_unref(kr6->r.labelid);
free(kr6);
return (-1);
}
@@ -638,6 +640,7 @@ krVPN4_change(struct ktable *kt, struct
kr->r.ifindex = kl->ifindex;
if (kroute_insert(kt, kr) == -1) {
+ rtlabel_unref(kr->r.labelid);
free(kr);
return (-1);
}
@@ -713,6 +716,7 @@ krVPN6_change(struct ktable *kt, struct
kr6->r.ifindex = kl->ifindex;
if (kroute6_insert(kt, kr6) == -1) {
+ rtlabel_unref(kr6->r.labelid);
free(kr6);
return (-1);
}
@@ -777,8 +781,6 @@ kr_flush(u_int rtableid)
if ((kr->r.flags & F_BGPD_INSERTED)) {
if (kt->fib_sync) /* coupled */
send_rtmsg(kr_state.fd, RTM_DELETE, kt, &kr->r);
- rtlabel_unref(kr->r.labelid);
-
if (kroute_remove(kt, kr) == -1)
return (-1);
}
@@ -787,8 +789,6 @@ kr_flush(u_int rtableid)
if (kt->fib_sync) /* coupled */
send_rt6msg(kr_state.fd, RTM_DELETE, kt,
&kr6->r);
- rtlabel_unref(kr6->r.labelid);
-
if (kroute6_remove(kt, kr6) == -1)
return (-1);
}
@@ -812,8 +812,6 @@ kr4_delete(struct ktable *kt, struct kro
if (send_rtmsg(kr_state.fd, RTM_DELETE, kt, &kr->r) == -1)
return (-1);
- rtlabel_unref(kr->r.labelid);
-
if (kroute_remove(kt, kr) == -1)
return (-1);
@@ -835,8 +833,6 @@ kr6_delete(struct ktable *kt, struct kro
if (send_rt6msg(kr_state.fd, RTM_DELETE, kt, &kr6->r) == -1)
return (-1);
- rtlabel_unref(kr6->r.labelid);
-
if (kroute6_remove(kt, kr6) == -1)
return (-1);
@@ -858,8 +854,6 @@ krVPN4_delete(struct ktable *kt, struct
if (send_rtmsg(kr_state.fd, RTM_DELETE, kt, &kr->r) == -1)
return (-1);
- rtlabel_unref(kr->r.labelid);
-
if (kroute_remove(kt, kr) == -1)
return (-1);
@@ -881,8 +875,6 @@ krVPN6_delete(struct ktable *kt, struct
if (send_rt6msg(kr_state.fd, RTM_DELETE, kt, &kr6->r) == -1)
return (-1);
- rtlabel_unref(kr6->r.labelid);
-
if (kroute6_remove(kt, kr6) == -1)
return (-1);
@@ -1895,10 +1887,13 @@ kroute_remove(struct ktable *kt, struct
if (kr->r.flags & F_CONNECTED)
if (kif_kr_remove(kr) == -1) {
+ rtlabel_unref(kr->r.labelid);
free(kr);
return (-1);
}
+ rtlabel_unref(kr->r.labelid);
+
free(kr);
return (0);
}
@@ -2048,10 +2043,13 @@ kroute6_remove(struct ktable *kt, struct
if (kr->r.flags & F_CONNECTED)
if (kif_kr6_remove(kr) == -1) {
+ rtlabel_unref(kr->r.labelid);
free(kr);
return (-1);
}
+ rtlabel_unref(kr->r.labelid);
+
free(kr);
return (0);
}
@@ -3264,11 +3262,8 @@ fetchtable(struct ktable *kt)
else
kr->r.prefixlen =
prefixlen_classful(kr->r.prefix.s_addr);
- rtlabel_unref(kr->r.labelid);
- kr->r.labelid = 0;
if ((label = (struct sockaddr_rtlabel *)
rti_info[RTAX_LABEL]) != NULL) {
- kr->r.flags |= F_RTLABEL;
kr->r.labelid =
rtlabel_name2id(label->sr_label);
}
@@ -3305,11 +3300,8 @@ fetchtable(struct ktable *kt)
kr6->r.prefixlen = 128;
else
fatalx("INET6 route without netmask");
- rtlabel_unref(kr6->r.labelid);
- kr6->r.labelid = 0;
if ((label = (struct sockaddr_rtlabel *)
rti_info[RTAX_LABEL]) != NULL) {
- kr6->r.flags |= F_RTLABEL;
kr6->r.labelid =
rtlabel_name2id(label->sr_label);
}
@@ -3360,6 +3352,7 @@ fetchtable(struct ktable *kt)
if (sa->sa_family == AF_INET) {
if (rtm->rtm_priority == kr_state.fib_prio) {
send_rtmsg(kr_state.fd, RTM_DELETE, kt, &kr->r);
+ rtlabel_unref(kr->r.labelid);
free(kr);
} else
kroute_insert(kt, kr);
@@ -3367,6 +3360,7 @@ fetchtable(struct ktable *kt)
if (rtm->rtm_priority == kr_state.fib_prio) {
send_rt6msg(kr_state.fd, RTM_DELETE, kt,
&kr6->r);
+ rtlabel_unref(kr6->r.labelid);
free(kr6);
} else
kroute6_insert(kt, kr6);
@@ -3702,15 +3696,12 @@ dispatch_rtmsg_addr(struct rt_msghdr *rt
rtlabel_name2id(label->sr_label);
if (kr->r.labelid != new_labelid) {
rtlabel_unref(kr->r.labelid);
- kr->r.labelid = 0;
- flags |= F_RTLABEL;
kr->r.labelid = new_labelid;
rtlabel_changed = 1;
}
} else if (kr->r.labelid && label == NULL) {
rtlabel_unref(kr->r.labelid);
kr->r.labelid = 0;
- flags &= ~F_RTLABEL;
rtlabel_changed = 1;
}
@@ -3760,7 +3751,6 @@ add4:
kr->r.priority = prio;
if (label) {
- kr->r.flags |= F_RTLABEL;
kr->r.labelid =
rtlabel_name2id(label->sr_label);
}
@@ -3808,15 +3798,12 @@ add4:
rtlabel_name2id(label->sr_label);
if (kr6->r.labelid != new_labelid) {
rtlabel_unref(kr6->r.labelid);
- kr6->r.labelid = 0;
- flags |= F_RTLABEL;
kr6->r.labelid = new_labelid;
rtlabel_changed = 1;
}
} else if (kr6->r.labelid && label == NULL) {
rtlabel_unref(kr6->r.labelid);
kr6->r.labelid = 0;
- flags &= ~F_RTLABEL;
rtlabel_changed = 1;
}
@@ -3870,7 +3857,6 @@ add6:
kr6->r.priority = prio;
if (label) {
- kr6->r.flags |= F_RTLABEL;
kr6->r.labelid =
rtlabel_name2id(label->sr_label);
}
