`ifconfig mp* mplslabel N' validates the label both in ifconfig and each
driver's ioctl handler, but there is one case where all drivers install
a route without looking at the label at all.
SIOCSLIFPHYRTABLE in all three drivers just validates the rdomain and
sets the label to itself (0 after clone) such that the route is
(re)installed accordingly.
The problem is that respective routines, e.g. mpe_set_label(), do not
validate the label themselves but instead expect the callee to do so;
all of them do so except the SIOCSLIFPHYRTABLE ioctl handler case:
case SIOCSLIFPHYRTABLE:
if (ifr->ifr_rdomainid < 0 ||
...) {
error = EINVAL;
break;
}
if (sc->sc_rdomain != ifr->ifr_rdomainid) {
error = mpe_set_label(sc, sc->sc_smpls.smpls_label,
ifr->ifr_rdomainid);
}
break;
Given a pristine setup:
# ifconfig mpw
mpw: no such interface
# route -n show -mpls
Routing tables
# route -nT2 show -mpls
Routing tables
that means we can install routes for the explicit NULL label in
non-default routing tables:
# ifconfig mpw0 tunneldomain 2
# ifconfig mpw0 | grep label
mpls: label (unset) pwe3 remote label (unset) nocw nofat
# route -nT2 show -mpls
Routing tables
MPLS:
In label Out label Op Gateway Flags Refs Use
Mtu Prio Interface
0 - POP fe:e1:ba:db:eb:89 UTl 0 0
- 1 mpw0
and can never clean them up without reboot:
# ifconfig mpw0 -tunneldomain
# route -nT2 show -mpls
Routing tables
MPLS:
In label Out label Op Gateway Flags Refs Use
Mtu Prio Interface
0 - POP fe:e1:ba:db:eb:89 UTl 0 0
- 1 (null)
# ifconfig mpw0 destroy
# route -nT2 show -mpls
Routing tables
MPLS:
In label Out label Op Gateway Flags Refs Use
Mtu Prio Interface
0 - POP fe:e1:ba:db:eb:89 UTl 0 0
- 1 (null)
My first approach was removing the label check from mp*_clone_destroy()
to unconditionally delete the respective route, but my understanding is
that we must not install such bogus routes to begin with.
So fix this by adding the inverted check to the mp*_set_label() routines
after updating the rdomain and before route reinstallation to skip those
nonexistent ones, i.e. to avoid installing bogus ones.
I'd argue this is better factored out into some label validating helper
function that can then be used from multiple callees, but I don't want
to touch so much code without a proper setup at hand -- plus, the diff
is much easier to understand and review this way.
Note that I've only tested this with ifconfig(8) and route(8), I have
no ldpd(8) or proper MPLS link to test with.
Feedback? Objections? OK?
Index: if_mpw.c
===================================================================
RCS file: /cvs/src/sys/net/if_mpw.c,v
retrieving revision 1.60
diff -u -p -r1.60 if_mpw.c
--- if_mpw.c 17 Mar 2021 14:30:08 -0000 1.60
+++ if_mpw.c 17 Mar 2021 15:10:57 -0000
@@ -172,6 +172,10 @@ mpw_set_route(struct mpw_softc *sc, uint
sc->sc_smpls.smpls_label = label;
sc->sc_rdomain = rdomain;
+ /* only install with a label or mpw_clone_destroy() will ignore it */
+ if (sc->sc_smpls.smpls_label == MPLS_LABEL2SHIM(0))
+ return 0;
+
error = rt_ifa_add(&sc->sc_ifa, RTF_MPLS|RTF_LOCAL,
smplstosa(&sc->sc_smpls), sc->sc_rdomain);
if (error != 0)
Index: if_mpe.c
===================================================================
RCS file: /cvs/src/sys/net/if_mpe.c,v
retrieving revision 1.98
diff -u -p -r1.98 if_mpe.c
--- if_mpe.c 20 Feb 2021 05:03:37 -0000 1.98
+++ if_mpe.c 17 Mar 2021 15:10:57 -0000
@@ -339,6 +339,10 @@ mpe_set_label(struct mpe_softc *sc, uint
sc->sc_smpls.smpls_label = label;
sc->sc_rdomain = rdomain;
+ /* only install with a label or mpe_clone_destroy() will ignore it */
+ if (sc->sc_smpls.smpls_label == MPLS_LABEL2SHIM(0))
+ return 0;
+
error = rt_ifa_add(&sc->sc_ifa, RTF_MPLS|RTF_LOCAL,
smplstosa(&sc->sc_smpls), sc->sc_rdomain);
if (error)
Index: if_mpip.c
===================================================================
RCS file: /cvs/src/sys/net/if_mpip.c,v
retrieving revision 1.14
diff -u -p -r1.14 if_mpip.c
--- if_mpip.c 17 Mar 2021 14:30:09 -0000 1.14
+++ if_mpip.c 17 Mar 2021 15:11:08 -0000
@@ -170,6 +170,10 @@ mpip_set_route(struct mpip_softc *sc, ui
sc->sc_smpls.smpls_label = shim;
sc->sc_rdomain = rdomain;
+ /* only install with a label or mpip_clone_destroy() will ignore it */
+ if (sc->sc_smpls.smpls_label == MPLS_LABEL2SHIM(0))
+ return 0;
+
error = rt_ifa_add(&sc->sc_ifa, RTF_MPLS | RTF_LOCAL,
smplstosa(&sc->sc_smpls), sc->sc_rdomain);
if (error) {
