kern/131536: kernel does allow manipulation of subnet routes

[Roy Marples]

>Number:         131536
>Category:       kern
>Synopsis:       kernel does allow manipulation of subnet routes
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Mon Feb 09 17:10:01 UTC 2009
>Closed-Date:
>Last-Modified:
>Originator:     Roy Marples
>Release:        FreeBSD-7.1
>Organization:
>Environment:
FreeBSD uberlaptop 7.1-RELEASE-p2 FreeBSD 7.1-RELEASE-p2 #2: Mon Feb  9 16:03:41 GMT 2009     root at uberlaptop:/usr/obj/usr/src/sys/GENERIC  i386
>Description:
When manipulating the automatically added kernel route, the IFA_ROUTE flag incorrectly remains on the interface address.
Also, if no IA has the IFA_ROUTE flag and a connected route already exists it should not be an error assigning the address.

This affects all BSD and was discussed at NetBSD here
http://mail-index.netbsd.org/tech-net/2008/12/03/msg000896.html
and patch commited here
http://www.netbsd.org/cgi-bin/query-pr-single.pl?number=40133
>How-To-Repeat:
This shell script demonstrates the problem

#!/bin/sh -x

nw=192.168.1.0
nm=24
ifp1=bge0
ifa1=192.168.1.10
ifp2=iwi0
ifa2=192.168.1.20
snr=192.168.1.0

# Configure ifa's
ifconfig $ifp1 alias $ifa1/$nm
ifconfig $ifp2 alias $ifa2/$nm

# Connect ifp2
route change $nw/$nm -ifp $ifp2

# Remove and then add ifa1. The add should fail - this is a bug.
ifconfig $ifp1 -alias $ifa1
ifconfig $ifp1 alias $ifa1/$nm
ifconfig $ifp1

# Remove all aliases now
route delete $nw/$nm
ifconfig $ifp1 -alias $ifa1
ifconfig $ifp2 -alias $ifa2

>Fix:
Attached shar contains two patches, cut against FreeBSD-7.1

Patch attached with submission follows:

# This is a shell archive.  Save it in a file, remove anything before
# this line, and then unpack it by entering "sh file".  Note, it may
# create directories; files and directories will be owned by you and
# have default permissions.
#
# This archive contains:
#
#	rtsock.diff
#	in.diff
#
echo x - rtsock.diff
sed 's/^X//' >rtsock.diff << '4db83498abbda21a07ce620dbb9f4808'
X--- sys/net/rtsock.c.orig	2009-02-09 11:16:58.000000000 +0000
X+++ sys/net/rtsock.c	2009-02-09 16:47:45.000000000 +0000
X@@ -315,7 +315,7 @@
X 	struct rtentry *rt = NULL;
X 	struct radix_node_head *rnh;
X 	struct rt_addrinfo info;
X-	int len, error = 0;
X+	int len, error = 0, ifa_route = 0;
X 	struct ifnet *ifp = NULL;
X 	struct sockaddr_in jail;
X 
X@@ -404,6 +404,9 @@
X 		if (error == 0) {
X 			RT_LOCK(saved_nrt);
X 			rt = saved_nrt;
X+			if (rt->rt_ifa != NULL &&
X+			    rt->rt_ifa->ifa_flags & IFA_ROUTE)
X+				rt->rt_ifa->ifa_flags &= ~IFA_ROUTE;
X 			goto report;
X 		}
X 		break;
X@@ -516,6 +519,12 @@
X 					senderr(error);
X 				RT_LOCK(rt);
X 			}
X+			if (rt->rt_ifa != NULL &&
X+			    rt->rt_ifa->ifa_flags & IFA_ROUTE)
X+			{
X+			    rt->rt_ifa->ifa_flags &= ~IFA_ROUTE;
X+			    ifa_route = 1;
X+			}
X 			if (info.rti_ifa != NULL &&
X 			    info.rti_ifa != rt->rt_ifa &&
X 			    rt->rt_ifa != NULL &&
X@@ -544,6 +553,10 @@
X 				rt->rt_flags = (rt->rt_flags &
X 				    ~rtm->rtm_fmask) |
X 				    (rtm->rtm_flags & rtm->rtm_fmask);
X+			if (ifa_route &&
X+			    rt->rt_ifa != NULL &&
X+			    !(rt->rt_flags & RTF_STATIC))
X+				rt->rt_ifa->ifa_flags |= IFA_ROUTE;
X 			rt_setmetrics(rtm->rtm_inits, &rtm->rtm_rmx,
X 					&rt->rt_rmx);
X 			rtm->rtm_index = rt->rt_ifp->if_index;
4db83498abbda21a07ce620dbb9f4808
echo x - in.diff
sed 's/^X//' >in.diff << '4fc6bf5d423ec08741fa36740ccce5fd'
X--- sys/netinet/in.c.orig	2009-02-09 16:49:43.000000000 +0000
X+++ sys/netinet/in.c	2009-02-09 11:42:40.000000000 +0000
X@@ -858,6 +858,12 @@
X 	error = rtinit(&target->ia_ifa, (int)RTM_ADD, flags);
X 	if (!error)
X 		target->ia_flags |= IFA_ROUTE;
X+	else if (error == EEXIST) {
X+		/*
X+		 * The fact that the route already exists is not an error
X+		 */
X+		error = 0;
X+	}
X 	return error;
X }
X 
4fc6bf5d423ec08741fa36740ccce5fd
exit



>Release-Note:
>Audit-Trail:
>Unformatted: