From: kn <kn@openbsd.org>
Date: Fri, 26 Mar 2021 19:00:21 +0000 (+0000)
Subject: Only install route with label, fix route leak on destroy
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=f8890659f54d7e5bcf7ca5d075a374798a41d309;p=openbsd

Only install route with label, fix route leak on destroy

ifconfig mp* mplslabel N" validates the label both in ifconfig(8) 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) such that the route is (re)installed accordingly.

None of the driver's helper functions dealing with labels and routes
validate labels themselves but instead expect the callees, e.g. the ioctl
handler to do so.

That means we can install routes for the explicit NULL label in non-default
routing tables but are never able to clean them up without reboot.

Fix this by adding the inverse of mp*_clone_destroy()'s label check to the
routines installing the MPLS route to avoid bogus ones in the first place.

OK claudio
---

diff --git a/sys/net/if_mpe.c b/sys/net/if_mpe.c
index 5d14bfc440f..5c912d73d6b 100644
--- a/sys/net/if_mpe.c
+++ b/sys/net/if_mpe.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: if_mpe.c,v 1.99 2021/03/18 14:47:17 kn Exp $ */
+/* $OpenBSD: if_mpe.c,v 1.100 2021/03/26 19:00:21 kn Exp $ */
 
 /*
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@spootnik.org>
@@ -339,6 +339,10 @@ mpe_set_label(struct mpe_softc *sc, uint32_t label, unsigned int rdomain)
 	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)
diff --git a/sys/net/if_mpip.c b/sys/net/if_mpip.c
index 0929a5ed33b..a8daeeea314 100644
--- a/sys/net/if_mpip.c
+++ b/sys/net/if_mpip.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_mpip.c,v 1.14 2021/03/17 14:30:09 kn Exp $ */
+/*	$OpenBSD: if_mpip.c,v 1.15 2021/03/26 19:00:21 kn Exp $ */
 
 /*
  * Copyright (c) 2015 Rafael Zalamena <rzalamena@openbsd.org>
@@ -170,6 +170,10 @@ mpip_set_route(struct mpip_softc *sc, uint32_t shim, unsigned int rdomain)
 	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) {
diff --git a/sys/net/if_mpw.c b/sys/net/if_mpw.c
index e9538502bf9..f17693dc766 100644
--- a/sys/net/if_mpw.c
+++ b/sys/net/if_mpw.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_mpw.c,v 1.61 2021/03/17 18:53:25 kn Exp $ */
+/*	$OpenBSD: if_mpw.c,v 1.62 2021/03/26 19:00:21 kn Exp $ */
 
 /*
  * Copyright (c) 2015 Rafael Zalamena <rzalamena@openbsd.org>
@@ -172,6 +172,10 @@ mpw_set_route(struct mpw_softc *sc, uint32_t label, unsigned int rdomain)
 	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)