Make rt_mpls_set() be more strict in what it accepts. Also ensure that

the RTF_MPLS can't be toggled without rt_mpls_set() being called. While
RTF_MPLS is part of RTF_FMASK it should be excluded from the flags and mask
when they are applied to the route since toggling it requires a call to
rt_mpls_set().
OK bluhm@

Reported-by: syzbot+86344a9e31c27aa6f15b@syzkaller.appspotmail.com

2 files changed, 16 insertions, 10 deletions

diff --git a/sys/net/route.c b/sys/net/route.c
index 512d554dc41..d6fa43f979b 100644
--- a/sys/net/route.c
+++ b/sys/net/route.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: route.c,v 1.383 2019/03/03 16:31:12 deraadt Exp $	*/
+/*	$OpenBSD: route.c,v 1.384 2019/05/11 16:47:02 claudio Exp $	*/
 /*	$NetBSD: route.c,v 1.14 1996/02/13 22:00:46 christos Exp $	*/
 
 /*
@@ -1474,6 +1474,13 @@ rt_mpls_set(struct rtentry *rt, struct sockaddr *src, uint8_t op)
 	struct sockaddr_mpls	*psa_mpls = (struct sockaddr_mpls *)src;
 	struct rt_mpls		*rt_mpls;
 
+	if (psa_mpls == NULL && op != MPLS_OP_POP)
+		return (EOPNOTSUPP);
+	if (psa_mpls != NULL && psa_mpls->smpls_len != sizeof(*psa_mpls))
+		return (EINVAL);
+	if (psa_mpls != NULL && psa_mpls->smpls_family != AF_MPLS)
+		return (EAFNOSUPPORT);
+
 	rt->rt_llinfo = malloc(sizeof(struct rt_mpls), M_TEMP, M_NOWAIT|M_ZERO);
 	if (rt->rt_llinfo == NULL)
 		return (ENOMEM);
@@ -1481,9 +1488,7 @@ rt_mpls_set(struct rtentry *rt, struct sockaddr *src, uint8_t op)
 	rt_mpls = (struct rt_mpls *)rt->rt_llinfo;
 	if (psa_mpls != NULL)
 		rt_mpls->mpls_label = psa_mpls->smpls_label;
-
 	rt_mpls->mpls_operation = op;
-
 	/* XXX: set experimental bits */
 	rt->rt_flags |= RTF_MPLS;
 
diff --git a/sys/net/rtsock.c b/sys/net/rtsock.c
index 2f21cc6258a..ee88a011992 100644
--- a/sys/net/rtsock.c
+++ b/sys/net/rtsock.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rtsock.c,v 1.285 2019/04/05 12:58:34 bluhm Exp $	*/
+/*	$OpenBSD: rtsock.c,v 1.286 2019/05/11 16:47:02 claudio Exp $	*/
 /*	$NetBSD: rtsock.c,v 1.18 1996/03/29 00:32:10 cgd Exp $	*/
 
 /*
@@ -1069,16 +1069,14 @@ change:
 					break;
 			}
 #ifdef MPLS
-			if ((rtm->rtm_flags & RTF_MPLS) &&
-			    info->rti_info[RTAX_SRC] != NULL) {
+			if (rtm->rtm_flags & RTF_MPLS) {
 				NET_LOCK();
 				error = rt_mpls_set(rt,
 				    info->rti_info[RTAX_SRC], info->rti_mpls);
 				NET_UNLOCK();
 				if (error)
 					break;
-			} else if (newgate || ((rtm->rtm_fmask & RTF_MPLS) &&
-			    !(rtm->rtm_flags & RTF_MPLS))) {
+			} else if (newgate || (rtm->rtm_fmask & RTF_MPLS)) {
 				NET_LOCK();
 				/* if gateway changed remove MPLS information */
 				rt_mpls_clear(rt);
@@ -1098,11 +1096,14 @@ change:
 
 			NET_LOCK();
 			/* Hack to allow some flags to be toggled */
-			if (rtm->rtm_fmask)
+			if (rtm->rtm_fmask) {
+				/* MPLS flag it is set by rt_mpls_set() */
+				rtm->rtm_fmask &= ~RTF_MPLS;
+				rtm->rtm_flags &= ~RTF_MPLS;
 				rt->rt_flags =
 				    (rt->rt_flags & ~rtm->rtm_fmask) |
 				    (rtm->rtm_flags & rtm->rtm_fmask);
-
+			}
 			rtm_setmetrics(rtm->rtm_inits, &rtm->rtm_rmx,
 			    &rt->rt_rmx);