Change semantics for rnh_lookup() function: now

it performs exact match search, regardless of netmask existance.
This simplifies most of rnh_lookup() consumers.

Fix panic triggered by deleting non-existent host route.

PR:		kern/185092
Submitted by:	Nikolay Denev <ndenev at gmail.com>
MFC after:	1 month

sys/net/radix.c

@@ -208,24 +208,50 @@ rn_refines(void *m_arg, void *n_arg)
 	return (!masks_are_equal);
 }
 
+/*
+ * Search for exact match in given @head.
+ * Assume host bits are cleared in @v_arg if @m_arg is not NULL
+ * Note that prefixes with /32 or /128 masks are treated differently
+ * from host routes.
+ */
 struct radix_node *
 rn_lookup(void *v_arg, void *m_arg, struct radix_node_head *head)
 {
 	struct radix_node *x;
-	caddr_t netmask = 0;
+	caddr_t netmask;
 
-	if (m_arg) {
+	if (m_arg != NULL) {
+		/*
+		 * Most common case: search exact prefix/mask
+		 */
 		x = rn_addmask(m_arg, head->rnh_masks, 1,
 		    head->rnh_treetop->rn_offset);
-		if (x == 0)
-			return (0);
+		if (x == NULL)
+			return (NULL);
 		netmask = x->rn_key;
-	}
-	x = rn_match(v_arg, head);
-	if (x && netmask) {
-		while (x && x->rn_mask != netmask)
+
+		x = rn_match(v_arg, head);
+
+		while (x != NULL && x->rn_mask != netmask)
 			x = x->rn_dupedkey;
+
+		return (x);
 	}
+
+	/*
+	 * Search for host address.
+	 */
+	if ((x = rn_match(v_arg, head)) == NULL)
+		return (NULL);
+
+	/* Check if found key is the same */
+	if (LEN(x->rn_key) != LEN(v_arg) || bcmp(x->rn_key, v_arg, LEN(v_arg)))
+		return (NULL);
+
+	/* Check if this is not host route */
+	if (x->rn_mask != NULL)
+		return (NULL);
+
 	return (x);
 }
 
@@ -247,6 +273,9 @@ rn_satisfies_leaf(char *trial, struct radix_node *leaf, int skip)
 	return (1);
 }
 
+/*
+ * Search for longest-prefix match in given @head
+ */
 struct radix_node *
 rn_match(void *v_arg, struct radix_node_head *head)
 {

sys/net/radix.h

@@ -112,9 +112,9 @@ struct radix_node_head {
 		     struct radix_node_head *head, struct radix_node nodes[]);
 	struct	radix_node *(*rnh_deladdr)	/* remove based on sockaddr */
 		(void *v, void *mask, struct radix_node_head *head);
-	struct	radix_node *(*rnh_matchaddr)	/* locate based on sockaddr */
+	struct	radix_node *(*rnh_matchaddr)	/* longest match for sockaddr */
 		(void *v, struct radix_node_head *head);
-	struct	radix_node *(*rnh_lookup)	/* locate based on sockaddr */
+	struct	radix_node *(*rnh_lookup)	/*exact match for sockaddr*/
 		(void *v, void *mask, struct radix_node_head *head);
 	int	(*rnh_walktree)			/* traverse tree */
 		(struct radix_node_head *head, walktree_f_t *f, void *w);

sys/net/radix_mpath.c

@@ -151,6 +151,7 @@ rt_mpath_deldup(struct rtentry *headrt, struct rtentry *rt)
 
 /*
  * check if we have the same key/mask/gateway on the table already.
+ * Assume @rt rt_key host bits are cleared according to @netmask
  */
 int
 rt_mpath_conflict(struct radix_node_head *rnh, struct rtentry *rt,
@@ -158,76 +159,13 @@ rt_mpath_conflict(struct radix_node_head *rnh, struct rtentry *rt,
 {
 	struct radix_node *rn, *rn1;
 	struct rtentry *rt1;
-	char *p, *q, *eq;
-	int same, l, skip;
 
 	rn = (struct radix_node *)rt;
 	rn1 = rnh->rnh_lookup(rt_key(rt), netmask, rnh);
 	if (!rn1 || rn1->rn_flags & RNF_ROOT)
-		return 0;
+		return (0);
 
-	/*
-	 * unlike other functions we have in this file, we have to check
-	 * all key/mask/gateway as rnh_lookup can match less specific entry.
-	 */
-	rt1 = (struct rtentry *)rn1;
-
-	/* compare key. */
-	if (rt_key(rt1)->sa_len != rt_key(rt)->sa_len ||
-	    bcmp(rt_key(rt1), rt_key(rt), rt_key(rt1)->sa_len))
-		goto different;
-
-	/* key was the same.  compare netmask.  hairy... */
-	if (rt_mask(rt1) && netmask) {
-		skip = rnh->rnh_treetop->rn_offset;
-		if (rt_mask(rt1)->sa_len > netmask->sa_len) {
-			/*
-			 * as rt_mask(rt1) is made optimal by radix.c,
-			 * there must be some 1-bits on rt_mask(rt1)
-			 * after netmask->sa_len.  therefore, in
-			 * this case, the entries are different.
-			 */
-			if (rt_mask(rt1)->sa_len > skip)
-				goto different;
-			else {
-				/* no bits to compare, i.e. same*/
-				goto maskmatched;
-			}
-		}
-
-		l = rt_mask(rt1)->sa_len;
-		if (skip > l) {
-			/* no bits to compare, i.e. same */
-			goto maskmatched;
-		}
-		p = (char *)rt_mask(rt1);
-		q = (char *)netmask;
-		if (bcmp(p + skip, q + skip, l - skip))
-			goto different;
-		/*
-		 * need to go through all the bit, as netmask is not
-		 * optimal and can contain trailing 0s
-		 */
-		eq = (char *)netmask + netmask->sa_len;
-		q += l;
-		same = 1;
-		while (eq > q)
-			if (*q++) {
-				same = 0;
-				break;
-			}
-		if (!same)
-			goto different;
-	} else if (!rt_mask(rt1) && !netmask)
-		; /* no mask to compare, i.e. same */
-	else {
-		/* one has mask and the other does not, different */
-		goto different;
-	}
-
-maskmatched:
-
-	/* key/mask were the same.  compare gateway for all multipaths */
+	/* key/mask are the same. compare gateway for all multipaths */
 	do {
 		rt1 = (struct rtentry *)rn1;
 
@@ -248,11 +186,10 @@ rt_mpath_conflict(struct radix_node_head *rnh, struct rtentry *rt,
 		}
 
 		/* all key/mask/gateway are the same.  conflicting entry. */
-		return EEXIST;
+		return (EEXIST);
 	} while ((rn1 = rn_mpath_next(rn1)) != NULL);
 
-different:
-	return 0;
+	return (0);
 }
 
 void

sys/net/route.c

@@ -930,6 +930,57 @@ rtexpunge(struct rtentry *rt)
 	return (error);
 }
 
+#if 0
+int p_sockaddr(char *buf, int buflen, struct sockaddr *s);
+int rt_print(char *buf, int buflen, struct rtentry *rt);
+
+int
+p_sockaddr(char *buf, int buflen, struct sockaddr *s)
+{
+	void *paddr = NULL;
+
+	switch (s->sa_family) {
+	case AF_INET:
+		paddr = &((struct sockaddr_in *)s)->sin_addr;
+		break;
+	case AF_INET6:
+		paddr = &((struct sockaddr_in6 *)s)->sin6_addr;
+		break;
+	}
+
+	if (paddr == NULL)
+		return (0);
+
+	if (inet_ntop(s->sa_family, paddr, buf, buflen) == NULL)
+		return (0);
+	
+	return (strlen(buf));
+}
+
+int
+rt_print(char *buf, int buflen, struct rtentry *rt)
+{
+	struct sockaddr *addr, *mask;
+	int i = 0;
+
+	addr = rt_key(rt);
+	mask = rt_mask(rt);
+
+	i = p_sockaddr(buf, buflen, addr);
+	if (!(rt->rt_flags & RTF_HOST)) {
+		buf[i++] = '/';
+		i += p_sockaddr(buf + i, buflen - i, mask);
+	}
+
+	if (rt->rt_flags & RTF_GATEWAY) {
+		buf[i++] = '>';
+		i += p_sockaddr(buf + i, buflen - i, rt->rt_gateway);
+	}
+
+	return (i);
+}
+#endif
+
 #ifdef RADIX_MPATH
 static int
 rn_mpath_update(int req, struct rt_addrinfo *info,
@@ -943,10 +994,11 @@ rn_mpath_update(int req, struct rt_addrinfo *info,
 	register struct radix_node *rn;
 	int error = 0;
 
-	rn = rnh->rnh_matchaddr(dst, rnh);
+	rn = rnh->rnh_lookup(dst, netmask, rnh);
 	if (rn == NULL)
 		return (ESRCH);
 	rto = rt = RNTORT(rn);
+
 	rt = rt_mpath_matchgate(rt, gateway);
 	if (rt == NULL)
 		return (ESRCH);
@@ -1521,8 +1573,7 @@ rtinit1(struct ifaddr *ifa, int cmd, int flags, int fibnum)
 			rn = rnh->rnh_lookup(dst, netmask, rnh);
 			error = (rn == NULL ||
 			    (rn->rn_flags & RNF_ROOT) ||
-			    RNTORT(rn)->rt_ifa != ifa ||
-			    !sa_equal((struct sockaddr *)rn->rn_key, dst));
+			    RNTORT(rn)->rt_ifa != ifa);
 			RADIX_NODE_HEAD_RUNLOCK(rnh);
 			if (error) {
 				/* this is only an error if bad on ALL tables */

sys/net/rtsock.c

@@ -727,10 +727,24 @@ route_output(struct mbuf *m, struct socket *so)
 		    info.rti_info[RTAX_DST]->sa_family);
 		if (rnh == NULL)
 			senderr(EAFNOSUPPORT);
+
 		RADIX_NODE_HEAD_RLOCK(rnh);
-		rt = (struct rtentry *) rnh->rnh_lookup(info.rti_info[RTAX_DST],
-			info.rti_info[RTAX_NETMASK], rnh);
-		if (rt == NULL) {	/* XXX looks bogus */
+
+		if (info.rti_info[RTAX_NETMASK] == NULL &&
+		    rtm->rtm_type == RTM_GET) {
+			/*
+			 * Provide logest prefix match for
+			 * address lookup (no mask).
+			 * 'route -n get addr'
+			 */
+			rt = (struct rtentry *) rnh->rnh_matchaddr(
+			    info.rti_info[RTAX_DST], rnh);
+		} else
+			rt = (struct rtentry *) rnh->rnh_lookup(
+			    info.rti_info[RTAX_DST],
+			    info.rti_info[RTAX_NETMASK], rnh);
+
+		if (rt == NULL) {
 			RADIX_NODE_HEAD_RUNLOCK(rnh);
 			senderr(ESRCH);
 		}
@@ -787,25 +801,6 @@ route_output(struct mbuf *m, struct socket *so)
 		RT_ADDREF(rt);
 		RADIX_NODE_HEAD_RUNLOCK(rnh);
 
-		/* 
-		 * Fix for PR: 82974
-		 *
-		 * RTM_CHANGE/LOCK need a perfect match, rn_lookup()
-		 * returns a perfect match in case a netmask is
-		 * specified.  For host routes only a longest prefix
-		 * match is returned so it is necessary to compare the
-		 * existence of the netmask.  If both have a netmask
-		 * rnh_lookup() did a perfect match and if none of them
-		 * have a netmask both are host routes which is also a
-		 * perfect match.
-		 */
-
-		if (rtm->rtm_type != RTM_GET && 
-		    (!rt_mask(rt) != !info.rti_info[RTAX_NETMASK])) {
-			RT_UNLOCK(rt);
-			senderr(ESRCH);
-		}
-
 		switch(rtm->rtm_type) {
 
 		case RTM_GET: