Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 1 Nov 2013 16:47:20 +0400
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        net@FreeBSD.org, current@FreeBSD.org
Subject:   [CFT & review] new in_control()
Message-ID:  <20131101124720.GF52889@FreeBSD.org>

next in thread | raw e-mail | index | archive | help

--SvF6CGw9fzJC4Rcx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

  Hi!

  I've got a patch that cleans up the way we configure
and delete IPv4 on interfaces. What it does:

1) separate function for SIOCAIFADDR, with clear code
   flow from beginning to the end.
2) separate function for SIOCDIFADDR, with clear code
   flow from beginning to the end.
3) provided 1) and 2) the in_control() got very thin
   and clear.

The above wasn't just a cut&paste job, instead every
step taken was evaluated. I've cut quite a lot of strange
code, added extra sanity checking and provided comments
on the strange code that remains.

4) sx(9) lock covers entire SIOCAIFADDR/SIOCDIFADDR
   operation, so we close races ifconfig vs ifconfig,
   or ifconfig vs mpd.
   On interface detach SIOCDIFADDR is called w/o sx(9),
   but its operation is covered by IF_ADDR_LOCK().

Also, except of redesign of SIOCAIFADDR/SIOCDIFADDR,
the following two related changes leaked into the patch.
It is possible to separate them out, but won't be easy.

5) Removed useloopback conditional. Rationale:
   - option was always on since pre-FreeBSD times
   - sysctl knob lives in invalid (ethernet) namespace,
     and documented in wrong (arp(8)) place.
   - since new-ARP, the knob was consulted on route
     addition, but was ignored on delete.
   - operation of network stack useloopback=0 is
     strange

   The only reason running useloopback=0 could be
   a router that doesn't want to pollute large network
   with its /32 announces. However, this can be achieved
   with filtering in routing daemons.

6) Implemented correctly code from r201282, that tried
   to keep localhost route in table when multiple P2P
   interfaces with same local address are created and
   deleted.

The check in of the code can cause problems. I could make
mistakes, and some program that relied on strange behavior
can pop up. Thus, early testing is appreciated.

So far I have tested simple address assignment, CARP,
and mpd5 as L2TP access concentrator.

Advice for reviewers is to not look at diff, but look at
patched in.c instead.

-- 
Totus tuus, Glebius.

--SvF6CGw9fzJC4Rcx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="in_control.diff"

Index: sys/net/if.c
===================================================================
--- sys/net/if.c	(revision 257503)
+++ sys/net/if.c	(working copy)
@@ -1525,6 +1525,25 @@ ifa_del_loopback_route(struct ifaddr *ifa, struct
 	return (error);
 }
 
+int
+ifa_switch_loopback_route(struct ifaddr *ifa, struct sockaddr *sa)
+{
+	struct rtentry *rt;
+
+	rt = rtalloc1_fib(sa, 0, 0, 0);
+	if (rt == NULL) {
+		log(LOG_DEBUG, "%s: fail", __func__);
+		return (EHOSTUNREACH);
+	}
+	((struct sockaddr_dl *)rt->rt_gateway)->sdl_type =
+	    ifa->ifa_ifp->if_type;
+	((struct sockaddr_dl *)rt->rt_gateway)->sdl_index =
+	    ifa->ifa_ifp->if_index;
+	RTFREE_LOCKED(rt);
+
+	return (0);
+}
+
 /*
  * XXX: Because sockaddr_dl has deeper structure than the sockaddr
  * structs used to represent other address families, it is necessary
Index: sys/net/if_var.h
===================================================================
--- sys/net/if_var.h	(revision 257503)
+++ sys/net/if_var.h	(working copy)
@@ -491,6 +491,7 @@ struct	ifnet *ifunit_ref(const char *);
 
 int	ifa_add_loopback_route(struct ifaddr *, struct sockaddr *);
 int	ifa_del_loopback_route(struct ifaddr *, struct sockaddr *);
+int	ifa_switch_loopback_route(struct ifaddr *, struct sockaddr *);
 
 struct	ifaddr *ifa_ifwithaddr(struct sockaddr *);
 int		ifa_ifwithaddr_check(struct sockaddr *);
Index: sys/netinet/if_ether.c
===================================================================
--- sys/netinet/if_ether.c	(revision 257503)
+++ sys/netinet/if_ether.c	(working copy)
@@ -85,8 +85,6 @@ static SYSCTL_NODE(_net_link_ether, PF_ARP, arp, C
 static VNET_DEFINE(int, arpt_keep) = (20*60);	/* once resolved, good for 20
 						 * minutes */
 static VNET_DEFINE(int, arp_maxtries) = 5;
-VNET_DEFINE(int, useloopback) = 1;	/* use loopback interface for
-					 * local traffic */
 static VNET_DEFINE(int, arp_proxyall) = 0;
 static VNET_DEFINE(int, arpt_down) = 20;	/* keep incomplete entries for
 						 * 20 seconds */
@@ -111,9 +109,6 @@ SYSCTL_VNET_INT(_net_link_ether_inet, OID_AUTO, ma
 SYSCTL_VNET_INT(_net_link_ether_inet, OID_AUTO, maxtries, CTLFLAG_RW,
 	&VNET_NAME(arp_maxtries), 0,
 	"ARP resolution attempts before returning error");
-SYSCTL_VNET_INT(_net_link_ether_inet, OID_AUTO, useloopback, CTLFLAG_RW,
-	&VNET_NAME(useloopback), 0,
-	"Use the loopback interface for local traffic");
 SYSCTL_VNET_INT(_net_link_ether_inet, OID_AUTO, proxyall, CTLFLAG_RW,
 	&VNET_NAME(arp_proxyall), 0,
 	"Enable proxy ARP for all suitable requests");
Index: sys/netinet/in.c
===================================================================
--- sys/netinet/in.c	(revision 257503)
+++ sys/netinet/in.c	(working copy)
@@ -47,6 +47,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/proc.h>
 #include <sys/sysctl.h>
 #include <sys/syslog.h>
+#include <sys/sx.h>
 
 #include <net/if.h>
 #include <net/if_var.h>
@@ -71,10 +72,10 @@ static int in_mask2len(struct in_addr *);
 static void in_len2mask(struct in_addr *, int);
 static int in_lifaddr_ioctl(struct socket *, u_long, caddr_t,
 	struct ifnet *, struct thread *);
+static int in_aifaddr_ioctl(caddr_t, struct ifnet *, struct thread *);
+static int in_difaddr_ioctl(caddr_t, struct ifnet *, struct thread *);
 
 static void	in_socktrim(struct sockaddr_in *);
-static int	in_ifinit(struct ifnet *, struct in_ifaddr *,
-		    struct sockaddr_in *, int, int);
 static void	in_purgemaddrs(struct ifnet *);
 
 static VNET_DEFINE(int, nosameprefix);
@@ -86,6 +87,9 @@ SYSCTL_VNET_INT(_net_inet_ip, OID_AUTO, no_same_pr
 VNET_DECLARE(struct inpcbinfo, ripcbinfo);
 #define	V_ripcbinfo			VNET(ripcbinfo)
 
+static struct sx in_control_sx;
+SX_SYSINIT(in_control_sx, &in_control_sx, "in_control");
+
 /*
  * Return 1 if an internet address is for a ``local'' host
  * (one to which we have a connection).
@@ -128,6 +132,28 @@ in_localip(struct in_addr in)
 }
 
 /*
+ * Return an address equal to the supplied one, but not the same.
+ */
+static struct in_ifaddr *
+more_localip(struct in_ifaddr *ia)
+{
+	in_addr_t in = IA_SIN(ia)->sin_addr.s_addr;
+	struct in_ifaddr *it;
+
+	IN_IFADDR_RLOCK();
+	LIST_FOREACH(it, INADDR_HASH(in), ia_hash) {
+		if (it != ia && IA_SIN(it)->sin_addr.s_addr == in) {
+			ifa_ref(&it->ia_ifa);
+			IN_IFADDR_RUNLOCK();
+			return (it);
+		}
+	}
+	IN_IFADDR_RUNLOCK();
+
+	return (NULL);
+}
+
+/*
  * Determine whether an IP address is in a reserved set of addresses
  * that may not be forwarded, or whether datagrams to that destination
  * may be forwarded.
@@ -203,40 +229,22 @@ in_len2mask(struct in_addr *mask, int len)
 
 /*
  * Generic internet control operations (ioctl's).
- *
- * ifp is NULL if not an interface-specific ioctl.
  */
-/* ARGSUSED */
 int
 in_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp,
     struct thread *td)
 {
-	register struct ifreq *ifr = (struct ifreq *)data;
-	register struct in_ifaddr *ia, *iap;
-	register struct ifaddr *ifa;
-	struct in_addr allhosts_addr;
-	struct in_addr dst;
-	struct in_ifinfo *ii;
-	struct in_aliasreq *ifra = (struct in_aliasreq *)data;
-	int error, hostIsNew, iaIsNew, maskIsNew;
-	int iaIsFirst;
-	u_long ocmd = cmd;
+	struct ifreq *ifr = (struct ifreq *)data;
+	struct sockaddr_in *addr = (struct sockaddr_in *)&ifr->ifr_addr;
+	struct in_ifaddr *ia;
+	int error;
 
-	/*
-	 * Pre-10.x compat: OSIOCAIFADDR passes a shorter
-	 * struct in_aliasreq, without ifra_vhid.
-	 */
-	if (cmd == OSIOCAIFADDR)
-		cmd = SIOCAIFADDR;
+	if (ifp == NULL)
+		return (EADDRNOTAVAIL);
 
-	ia = NULL;
-	iaIsFirst = 0;
-	iaIsNew = 0;
-	allhosts_addr.s_addr = htonl(INADDR_ALLHOSTS_GROUP);
-
 	/*
-	 * Filter out ioctls we implement directly; forward the rest on to
-	 * in_lifaddr_ioctl() and ifp->if_ioctl().
+	 * Filter out 4 ioctls we implement directly.  Forward the rest
+	 * to specific functions and ifp->if_ioctl().
 	 */
 	switch (cmd) {
 	case SIOCGIFADDR:
@@ -243,34 +251,21 @@ in_control(struct socket *so, u_long cmd, caddr_t
 	case SIOCGIFBRDADDR:
 	case SIOCGIFDSTADDR:
 	case SIOCGIFNETMASK:
+		break;
 	case SIOCDIFADDR:
-		break;
+		sx_xlock(&in_control_sx);
+		error = in_difaddr_ioctl(data, ifp, td);
+		sx_xunlock(&in_control_sx);
+		return (error);
 	case SIOCAIFADDR:
-		/*
-		 * ifra_addr must be present and be of INET family.
-		 * ifra_broadaddr and ifra_mask are optional.
-		 */
-		if (ifra->ifra_addr.sin_len != sizeof(struct sockaddr_in) ||
-		    ifra->ifra_addr.sin_family != AF_INET)
-			return (EINVAL);
-		if (ifra->ifra_broadaddr.sin_len != 0 &&
-		    (ifra->ifra_broadaddr.sin_len !=
-		    sizeof(struct sockaddr_in) ||
-		    ifra->ifra_broadaddr.sin_family != AF_INET))
-			return (EINVAL);
-#if 0
-		/*
-		 * ifconfig(8) in pre-10.x doesn't set sin_family for the
-		 * mask. The code is disabled for the 10.x timeline, to
-		 * make SIOCAIFADDR compatible with 9.x ifconfig(8).
-		 * The code should be enabled in 11.x
-		 */
-		if (ifra->ifra_mask.sin_len != 0 &&
-		    (ifra->ifra_mask.sin_len != sizeof(struct sockaddr_in) ||
-		    ifra->ifra_mask.sin_family != AF_INET))
-			return (EINVAL);
-#endif
-		break;
+		sx_xlock(&in_control_sx);
+		error = in_aifaddr_ioctl(data, ifp, td);
+		sx_xunlock(&in_control_sx);
+		return (error);
+	case SIOCALIFADDR:
+	case SIOCDLIFADDR:
+	case SIOCGLIFADDR:
+		return (in_lifaddr_ioctl(so, cmd, data, ifp, td));
 	case SIOCSIFADDR:
 	case SIOCSIFBRDADDR:
 	case SIOCSIFDSTADDR:
@@ -277,306 +272,353 @@ in_control(struct socket *so, u_long cmd, caddr_t
 	case SIOCSIFNETMASK:
 		/* We no longer support that old commands. */
 		return (EINVAL);
-
-	case SIOCALIFADDR:
-		if (td != NULL) {
-			error = priv_check(td, PRIV_NET_ADDIFADDR);
-			if (error)
-				return (error);
-		}
-		if (ifp == NULL)
-			return (EINVAL);
-		return in_lifaddr_ioctl(so, cmd, data, ifp, td);
-
-	case SIOCDLIFADDR:
-		if (td != NULL) {
-			error = priv_check(td, PRIV_NET_DELIFADDR);
-			if (error)
-				return (error);
-		}
-		if (ifp == NULL)
-			return (EINVAL);
-		return in_lifaddr_ioctl(so, cmd, data, ifp, td);
-
-	case SIOCGLIFADDR:
-		if (ifp == NULL)
-			return (EINVAL);
-		return in_lifaddr_ioctl(so, cmd, data, ifp, td);
-
 	default:
-		if (ifp == NULL || ifp->if_ioctl == NULL)
+		if (ifp->if_ioctl == NULL)
 			return (EOPNOTSUPP);
 		return ((*ifp->if_ioctl)(ifp, cmd, data));
 	}
 
-	if (ifp == NULL)
-		return (EADDRNOTAVAIL);
-
 	/*
-	 * Security checks before we get involved in any work.
-	 */
-	switch (cmd) {
-	case SIOCAIFADDR:
-		if (td != NULL) {
-			error = priv_check(td, PRIV_NET_ADDIFADDR);
-			if (error)
-				return (error);
-		}
-		break;
-
-	case SIOCDIFADDR:
-		if (td != NULL) {
-			error = priv_check(td, PRIV_NET_DELIFADDR);
-			if (error)
-				return (error);
-		}
-		break;
-	}
-
-	/*
 	 * Find address for this interface, if it exists.
-	 *
-	 * If an alias address was specified, find that one instead of the
-	 * first one on the interface, if possible.
 	 */
-	dst = ((struct sockaddr_in *)&ifr->ifr_addr)->sin_addr;
 	IN_IFADDR_RLOCK();
-	LIST_FOREACH(iap, INADDR_HASH(dst.s_addr), ia_hash) {
-		if (iap->ia_ifp == ifp &&
-		    iap->ia_addr.sin_addr.s_addr == dst.s_addr) {
-			if (td == NULL || prison_check_ip4(td->td_ucred,
-			    &dst) == 0)
-				ia = iap;
+	LIST_FOREACH(ia, INADDR_HASH(addr->sin_addr.s_addr), ia_hash) {
+		if (ia->ia_ifp == ifp &&
+		    ia->ia_addr.sin_addr.s_addr == addr->sin_addr.s_addr &&
+		    prison_check_ip4(td->td_ucred, &addr->sin_addr) == 0)
 			break;
-		}
 	}
-	if (ia != NULL)
-		ifa_ref(&ia->ia_ifa);
-	IN_IFADDR_RUNLOCK();
+
 	if (ia == NULL) {
-		IF_ADDR_RLOCK(ifp);
-		TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
-			iap = ifatoia(ifa);
-			if (iap->ia_addr.sin_family == AF_INET) {
-				if (td != NULL &&
-				    prison_check_ip4(td->td_ucred,
-				    &iap->ia_addr.sin_addr) != 0)
-					continue;
-				ia = iap;
-				break;
-			}
-		}
-		if (ia != NULL)
-			ifa_ref(&ia->ia_ifa);
-		IF_ADDR_RUNLOCK(ifp);
+		IN_IFADDR_RUNLOCK();
+		return (EADDRNOTAVAIL);
 	}
-	if (ia == NULL)
-		iaIsFirst = 1;
 
 	error = 0;
 	switch (cmd) {
-	case SIOCAIFADDR:
-	case SIOCDIFADDR:
-		if (ifra->ifra_addr.sin_family == AF_INET) {
-			struct in_ifaddr *oia;
+	case SIOCGIFADDR:
+		*addr = ia->ia_addr;
+		break;
 
-			IN_IFADDR_RLOCK();
-			for (oia = ia; ia; ia = TAILQ_NEXT(ia, ia_link)) {
-				if (ia->ia_ifp == ifp  &&
-				    ia->ia_addr.sin_addr.s_addr ==
-				    ifra->ifra_addr.sin_addr.s_addr)
-					break;
-			}
-			if (ia != NULL && ia != oia)
-				ifa_ref(&ia->ia_ifa);
-			if (oia != NULL && ia != oia)
-				ifa_free(&oia->ia_ifa);
-			IN_IFADDR_RUNLOCK();
-			if ((ifp->if_flags & IFF_POINTOPOINT)
-			    && (cmd == SIOCAIFADDR)
-			    && (ifra->ifra_dstaddr.sin_addr.s_addr
-				== INADDR_ANY)) {
-				error = EDESTADDRREQ;
-				goto out;
-			}
+	case SIOCGIFBRDADDR:
+		if ((ifp->if_flags & IFF_BROADCAST) == 0) {
+			error = EINVAL;
+			break;
 		}
-		if (cmd == SIOCDIFADDR && ia == NULL) {
-			error = EADDRNOTAVAIL;
-			goto out;
-		}
-		if (ia == NULL) {
-			ifa = ifa_alloc(sizeof(struct in_ifaddr), M_WAITOK);
-			ia = (struct in_ifaddr *)ifa;
-			ifa->ifa_addr = (struct sockaddr *)&ia->ia_addr;
-			ifa->ifa_dstaddr = (struct sockaddr *)&ia->ia_dstaddr;
-			ifa->ifa_netmask = (struct sockaddr *)&ia->ia_sockmask;
+		*addr = ia->ia_broadaddr;
+		break;
 
-			ia->ia_sockmask.sin_len = 8;
-			ia->ia_sockmask.sin_family = AF_INET;
-			if (ifp->if_flags & IFF_BROADCAST) {
-				ia->ia_broadaddr.sin_len = sizeof(ia->ia_addr);
-				ia->ia_broadaddr.sin_family = AF_INET;
-			}
-			ia->ia_ifp = ifp;
-
-			ifa_ref(ifa);			/* if_addrhead */
-			IF_ADDR_WLOCK(ifp);
-			TAILQ_INSERT_TAIL(&ifp->if_addrhead, ifa, ifa_link);
-			IF_ADDR_WUNLOCK(ifp);
-			ifa_ref(ifa);			/* in_ifaddrhead */
-			IN_IFADDR_WLOCK();
-			TAILQ_INSERT_TAIL(&V_in_ifaddrhead, ia, ia_link);
-			IN_IFADDR_WUNLOCK();
-			iaIsNew = 1;
+	case SIOCGIFDSTADDR:
+		if ((ifp->if_flags & IFF_POINTOPOINT) == 0) {
+			error = EINVAL;
+			break;
 		}
+		*addr = ia->ia_dstaddr;
 		break;
 
-	case SIOCGIFADDR:
 	case SIOCGIFNETMASK:
-	case SIOCGIFDSTADDR:
-	case SIOCGIFBRDADDR:
-		if (ia == NULL) {
-			error = EADDRNOTAVAIL;
-			goto out;
-		}
+		*addr = ia->ia_sockmask;
 		break;
 	}
 
+	IN_IFADDR_RUNLOCK();
+
+	return (error);
+}
+
+static int
+in_aifaddr_ioctl(caddr_t data, struct ifnet *ifp, struct thread *td)
+{
+	const struct in_aliasreq *ifra = (struct in_aliasreq *)data;
+	const struct sockaddr_in *addr = &ifra->ifra_addr;
+	const struct sockaddr_in *broadaddr = &ifra->ifra_broadaddr;
+	const struct sockaddr_in *mask = &ifra->ifra_mask;
+	const struct sockaddr_in *dstaddr = &ifra->ifra_dstaddr;
+	const int vhid = ifra->ifra_vhid;
+	struct ifaddr *ifa;
+	struct in_ifaddr *ia;
+	bool iaIsFirst;
+	int error = 0;
+
+	error = priv_check(td, PRIV_NET_ADDIFADDR);
+	if (error)
+		return (error);
+
 	/*
-	 * Most paths in this switch return directly or via out.  Only paths
-	 * that remove the address break in order to hit common removal code.
+	 * ifra_addr must be present and be of INET family.
+	 * ifra_broadaddr/ifra_dstaddr and ifra_mask are optional.
 	 */
-	switch (cmd) {
-	case SIOCGIFADDR:
-		*((struct sockaddr_in *)&ifr->ifr_addr) = ia->ia_addr;
-		goto out;
+	if (addr->sin_len != sizeof(struct sockaddr_in) ||
+	    addr->sin_family != AF_INET)
+		return (EINVAL);
+	if (broadaddr->sin_len != 0 &&
+	    (broadaddr->sin_len != sizeof(struct sockaddr_in) ||
+	    broadaddr->sin_family != AF_INET))
+		return (EINVAL);
+	if (mask->sin_len != 0 &&
+	    (mask->sin_len != sizeof(struct sockaddr_in) ||
+	    mask->sin_family != AF_INET))
+		return (EINVAL);
+	if ((ifp->if_flags & IFF_POINTOPOINT) &&
+	    (dstaddr->sin_len != sizeof(struct sockaddr_in) ||
+	     dstaddr->sin_addr.s_addr == INADDR_ANY))
+		return (EDESTADDRREQ);
+	if (vhid > 0 && carp_attach_p == NULL)
+		return (EPROTONOSUPPORT);
 
-	case SIOCGIFBRDADDR:
-		if ((ifp->if_flags & IFF_BROADCAST) == 0) {
-			error = EINVAL;
-			goto out;
-		}
-		*((struct sockaddr_in *)&ifr->ifr_dstaddr) = ia->ia_broadaddr;
-		goto out;
+	/*
+	 * See whether address already exist.
+	 */
+	iaIsFirst = true;
+	ia = NULL;
+	IF_ADDR_RLOCK(ifp);
+	TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
+		struct in_ifaddr *it = ifatoia(ifa);
 
-	case SIOCGIFDSTADDR:
-		if ((ifp->if_flags & IFF_POINTOPOINT) == 0) {
-			error = EINVAL;
-			goto out;
-		}
-		*((struct sockaddr_in *)&ifr->ifr_dstaddr) = ia->ia_dstaddr;
-		goto out;
+		if (it->ia_addr.sin_family != AF_INET)
+			continue;
 
-	case SIOCGIFNETMASK:
-		*((struct sockaddr_in *)&ifr->ifr_addr) = ia->ia_sockmask;
-		goto out;
+		iaIsFirst = false;
+		if (it->ia_addr.sin_addr.s_addr == addr->sin_addr.s_addr &&
+		    prison_check_ip4(td->td_ucred, &addr->sin_addr) == 0)
+			ia = it;
+	}
+	IF_ADDR_RUNLOCK(ifp);
 
-	case SIOCAIFADDR:
-		maskIsNew = 0;
-		hostIsNew = 1;
-		error = 0;
-		if (ifra->ifra_addr.sin_addr.s_addr ==
-			    ia->ia_addr.sin_addr.s_addr)
-			hostIsNew = 0;
-		if (ifra->ifra_mask.sin_len) {
-			/*
-			 * QL: XXX
-			 * Need to scrub the prefix here in case
-			 * the issued command is SIOCAIFADDR with
-			 * the same address, but with a different
-			 * prefix length. And if the prefix length
-			 * is the same as before, then the call is
-			 * un-necessarily executed here.
-			 */
-			in_scrubprefix(ia, LLE_STATIC);
-			ia->ia_sockmask = ifra->ifra_mask;
-			ia->ia_sockmask.sin_family = AF_INET;
-			ia->ia_subnetmask =
-			    ntohl(ia->ia_sockmask.sin_addr.s_addr);
-			maskIsNew = 1;
-		}
-		if ((ifp->if_flags & IFF_POINTOPOINT) &&
-		    (ifra->ifra_dstaddr.sin_family == AF_INET)) {
-			in_scrubprefix(ia, LLE_STATIC);
-			ia->ia_dstaddr = ifra->ifra_dstaddr;
-			maskIsNew  = 1; /* We lie; but the effect's the same */
-		}
-		if (hostIsNew || maskIsNew)
-			error = in_ifinit(ifp, ia, &ifra->ifra_addr, maskIsNew,
-			    (ocmd == cmd ? ifra->ifra_vhid : 0));
-		if (error != 0 && iaIsNew)
-			break;
+	if (ia != NULL)
+		(void )in_difaddr_ioctl(data, ifp, td);
 
-		if ((ifp->if_flags & IFF_BROADCAST) &&
-		    ifra->ifra_broadaddr.sin_len)
-			ia->ia_broadaddr = ifra->ifra_broadaddr;
-		if (error == 0) {
-			ii = ((struct in_ifinfo *)ifp->if_afdata[AF_INET]);
-			if (iaIsFirst &&
-			    (ifp->if_flags & IFF_MULTICAST) != 0) {
-				error = in_joingroup(ifp, &allhosts_addr,
-				    NULL, &ii->ii_allhosts);
-			}
-			EVENTHANDLER_INVOKE(ifaddr_event, ifp);
-		}
-		goto out;
+	ifa = ifa_alloc(sizeof(struct in_ifaddr), M_WAITOK);
+	ia = (struct in_ifaddr *)ifa;
+	ifa->ifa_addr = (struct sockaddr *)&ia->ia_addr;
+	ifa->ifa_dstaddr = (struct sockaddr *)&ia->ia_dstaddr;
+	ifa->ifa_netmask = (struct sockaddr *)&ia->ia_sockmask;
 
-	case SIOCDIFADDR:
-		/*
-		 * in_scrubprefix() kills the interface route.
-		 */
-		in_scrubprefix(ia, LLE_STATIC);
+	ia->ia_ifp = ifp;
+	ia->ia_ifa.ifa_metric = ifp->if_metric;
+	ia->ia_addr = *addr;
+	if (mask->sin_len != 0) {
+		ia->ia_sockmask = *mask;
+		ia->ia_subnetmask = ntohl(ia->ia_sockmask.sin_addr.s_addr);
+	} else {
+		in_addr_t i = ntohl(addr->sin_addr.s_addr);
 
 		/*
-		 * in_ifadown gets rid of all the rest of
-		 * the routes.  This is not quite the right
-		 * thing to do, but at least if we are running
-		 * a routing process they will come back.
-		 */
-		in_ifadown(&ia->ia_ifa, 1);
-		EVENTHANDLER_INVOKE(ifaddr_event, ifp);
-		error = 0;
-		break;
+	 	 * Be compatible with network classes, if netmask isn't
+		 * supplied, guess it based on classes.
+	 	 */
+		if (IN_CLASSA(i))
+			ia->ia_subnetmask = IN_CLASSA_NET;
+		else if (IN_CLASSB(i))
+			ia->ia_subnetmask = IN_CLASSB_NET;
+		else
+			ia->ia_subnetmask = IN_CLASSC_NET;
+		ia->ia_sockmask.sin_addr.s_addr = htonl(ia->ia_subnetmask);
+	}
+	ia->ia_subnet = ntohl(addr->sin_addr.s_addr) & ia->ia_subnetmask;
+	in_socktrim(&ia->ia_sockmask);
 
-	default:
-		panic("in_control: unsupported ioctl");
+	if (ifp->if_flags & IFF_BROADCAST) {
+		if (broadaddr->sin_len != 0) {
+			ia->ia_broadaddr = *broadaddr;
+		} else if (ia->ia_subnetmask == IN_RFC3021_MASK) {
+			ia->ia_broadaddr.sin_addr.s_addr = INADDR_BROADCAST;
+			ia->ia_broadaddr.sin_len = sizeof(struct sockaddr_in);
+			ia->ia_broadaddr.sin_family = AF_INET;
+		} else {
+			ia->ia_broadaddr.sin_addr.s_addr =
+			    htonl(ia->ia_subnet | ~ia->ia_subnetmask);
+			ia->ia_broadaddr.sin_len = sizeof(struct sockaddr_in);
+			ia->ia_broadaddr.sin_family = AF_INET;
+		}
 	}
 
+	if (ifp->if_flags & IFF_POINTOPOINT)
+		ia->ia_dstaddr = *dstaddr;
+
+	/* XXXGL: rtinit() needs this strange assignment. */
+	if (ifp->if_flags & IFF_LOOPBACK)
+                ia->ia_dstaddr = ia->ia_addr;
+
+	ifa_ref(ifa);			/* if_addrhead */
+	IF_ADDR_WLOCK(ifp);
+	TAILQ_INSERT_TAIL(&ifp->if_addrhead, ifa, ifa_link);
+	IF_ADDR_WUNLOCK(ifp);
+
+	ifa_ref(ifa);			/* in_ifaddrhead */
+	IN_IFADDR_WLOCK();
+	TAILQ_INSERT_TAIL(&V_in_ifaddrhead, ia, ia_link);
+	LIST_INSERT_HEAD(INADDR_HASH(ia->ia_addr.sin_addr.s_addr), ia, ia_hash);
+	IN_IFADDR_WUNLOCK();
+
+	if (vhid != 0)
+		error = (*carp_attach_p)(&ia->ia_ifa, vhid);
+	if (error)
+		goto fail1;
+
+	/*
+	 * Give the interface a chance to initialize
+	 * if this is its first address,
+	 * and to validate the address if necessary.
+	 */
+	if (ifp->if_ioctl != NULL)
+		error = (*ifp->if_ioctl)(ifp, SIOCSIFADDR, (caddr_t)ia);
+	if (error)
+		goto fail2;
+
+	/*
+	 * Add route for the network.
+	 */
+	if (vhid == 0) {
+		int flags = RTF_UP;
+
+		if (ifp->if_flags & (IFF_LOOPBACK|IFF_POINTOPOINT))
+			flags |= RTF_HOST;
+
+		error = in_addprefix(ia, flags);
+		if (error)
+			goto fail2;
+	}
+
+	/*
+	 * Add a loopback route to self.
+	 */
+	if (vhid == 0 && (ifp->if_flags & IFF_LOOPBACK) == 0 &&
+	    ia->ia_addr.sin_addr.s_addr != INADDR_ANY) {
+		struct in_ifaddr *eia;
+
+		eia = more_localip(ia);
+
+		if (eia == NULL) {
+			error = ifa_add_loopback_route((struct ifaddr *)ia,
+			    (struct sockaddr *)&ia->ia_addr);
+			if (error)
+				goto fail3;
+		} else
+			ifa_free(&eia->ia_ifa);
+	}
+
+	if (iaIsFirst && (ifp->if_flags & IFF_MULTICAST)) {
+		struct in_addr allhosts_addr;
+		struct in_ifinfo *ii;
+
+		ii = ((struct in_ifinfo *)ifp->if_afdata[AF_INET]);
+		allhosts_addr.s_addr = htonl(INADDR_ALLHOSTS_GROUP);
+
+		error = in_joingroup(ifp, &allhosts_addr, NULL,
+			&ii->ii_allhosts);
+	}
+
+	EVENTHANDLER_INVOKE(ifaddr_event, ifp);
+
+	return (error);
+
+fail3:
+	if (vhid == 0)
+		(void )in_scrubprefix(ia, LLE_STATIC);
+
+fail2:
 	if (ia->ia_ifa.ifa_carp)
 		(*carp_detach_p)(&ia->ia_ifa);
 
+fail1:
 	IF_ADDR_WLOCK(ifp);
-	/* Re-check that ia is still part of the list. */
+	TAILQ_REMOVE(&ifp->if_addrhead, &ia->ia_ifa, ifa_link);
+	IF_ADDR_WUNLOCK(ifp);
+	ifa_free(&ia->ia_ifa);
+
+	IN_IFADDR_WLOCK();
+	TAILQ_REMOVE(&V_in_ifaddrhead, ia, ia_link);
+	LIST_REMOVE(ia, ia_hash);
+	IN_IFADDR_WUNLOCK();
+	ifa_free(&ia->ia_ifa);
+
+	return (error);
+}
+
+static int
+in_difaddr_ioctl(caddr_t data, struct ifnet *ifp, struct thread *td)
+{
+	const struct ifreq *ifr = (struct ifreq *)data;
+	const struct sockaddr_in *addr = (struct sockaddr_in *)&ifr->ifr_addr;
+	struct ifaddr *ifa;
+	struct in_ifaddr *ia;
+	bool deleteAny, iaIsLast;
+	int error;
+
+	if (td != NULL) {
+		error = priv_check(td, PRIV_NET_DELIFADDR);
+		if (error)
+			return (error);
+	}
+
+	if (addr->sin_len != sizeof(struct sockaddr_in) ||
+	    addr->sin_family != AF_INET)
+		deleteAny = true;
+	else
+		deleteAny = false;
+
+	iaIsLast = true;
+	ia = NULL;
+	IF_ADDR_WLOCK(ifp);
 	TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
-		if (ifa == &ia->ia_ifa)
-			break;
+		struct in_ifaddr *it = ifatoia(ifa);
+
+		if (it->ia_addr.sin_family != AF_INET)
+			continue;
+
+		if (deleteAny && ia == NULL && (td == NULL ||
+		    prison_check_ip4(td->td_ucred, &it->ia_addr.sin_addr) == 0))
+			ia = it;
+
+		if (it->ia_addr.sin_addr.s_addr == addr->sin_addr.s_addr &&
+		    (td == NULL || prison_check_ip4(td->td_ucred,
+		    &addr->sin_addr) == 0))
+			ia = it;
+
+		if (it != ia)
+			iaIsLast = false;
 	}
-	if (ifa == NULL) {
-		/*
-		 * If we lost the race with another thread, there is no need to
-		 * try it again for the next loop as there is no other exit
-		 * path between here and out.
-		 */
+
+	if (ia == NULL) {
 		IF_ADDR_WUNLOCK(ifp);
-		error = EADDRNOTAVAIL;
-		goto out;
+		return (EADDRNOTAVAIL);
 	}
+
 	TAILQ_REMOVE(&ifp->if_addrhead, &ia->ia_ifa, ifa_link);
 	IF_ADDR_WUNLOCK(ifp);
-	ifa_free(&ia->ia_ifa);		      /* if_addrhead */
+	ifa_free(&ia->ia_ifa);		/* if_addrhead */
 
 	IN_IFADDR_WLOCK();
 	TAILQ_REMOVE(&V_in_ifaddrhead, ia, ia_link);
-
 	LIST_REMOVE(ia, ia_hash);
 	IN_IFADDR_WUNLOCK();
+	ifa_free(&ia->ia_ifa);		/* in_ifaddrhead */
+
 	/*
+	 * in_scrubprefix() kills the interface route.
+	 */
+	in_scrubprefix(ia, LLE_STATIC);
+
+	/*
+	 * in_ifadown gets rid of all the rest of
+	 * the routes.  This is not quite the right
+	 * thing to do, but at least if we are running
+	 * a routing process they will come back.
+	 */
+	in_ifadown(&ia->ia_ifa, 1);
+
+	if (ia->ia_ifa.ifa_carp)
+		(*carp_detach_p)(&ia->ia_ifa);
+
+	/*
 	 * If this is the last IPv4 address configured on this
 	 * interface, leave the all-hosts group.
 	 * No state-change report need be transmitted.
 	 */
-	IFP_TO_IA(ifp, iap);
-	if (iap == NULL) {
+	if (iaIsLast && (ifp->if_flags & IFF_MULTICAST)) {
+		struct in_ifinfo *ii;
+
 		ii = ((struct in_ifinfo *)ifp->if_afdata[AF_INET]);
 		IN_MULTI_LOCK();
 		if (ii->ii_allhosts) {
@@ -584,14 +626,11 @@ in_control(struct socket *so, u_long cmd, caddr_t
 			ii->ii_allhosts = NULL;
 		}
 		IN_MULTI_UNLOCK();
-	} else
-		ifa_free(&iap->ia_ifa);
+	}
 
-	ifa_free(&ia->ia_ifa);				/* in_ifaddrhead */
-out:
-	if (ia != NULL)
-		ifa_free(&ia->ia_ifa);
-	return (error);
+	EVENTHANDLER_INVOKE(ifaddr_event, ifp);
+
+	return (0);
 }
 
 /*
@@ -616,11 +655,23 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
 {
 	struct if_laddrreq *iflr = (struct if_laddrreq *)data;
 	struct ifaddr *ifa;
+	int error;
 
-	/* sanity checks */
-	if (data == NULL || ifp == NULL) {
-		panic("invalid argument to in_lifaddr_ioctl");
-		/*NOTRECHED*/
+	switch (cmd) {
+	case SIOCALIFADDR:
+		if (td != NULL) {
+			error = priv_check(td, PRIV_NET_ADDIFADDR);
+			if (error)
+				return (error);
+		}
+		break;
+	case SIOCDLIFADDR:
+		if (td != NULL) {
+			error = priv_check(td, PRIV_NET_DELIFADDR);
+			if (error)
+				return (error);
+		}
+		break;
 	}
 
 	switch (cmd) {
@@ -770,115 +821,6 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
 	return (EOPNOTSUPP);	/*just for safety*/
 }
 
-/*
- * Initialize an interface's internet address
- * and routing table entry.
- */
-static int
-in_ifinit(struct ifnet *ifp, struct in_ifaddr *ia, struct sockaddr_in *sin,
-    int masksupplied, int vhid)
-{
-	register u_long i = ntohl(sin->sin_addr.s_addr);
-	int flags, error = 0;
-
-	IN_IFADDR_WLOCK();
-	if (ia->ia_addr.sin_family == AF_INET)
-		LIST_REMOVE(ia, ia_hash);
-	ia->ia_addr = *sin;
-	LIST_INSERT_HEAD(INADDR_HASH(ia->ia_addr.sin_addr.s_addr),
-	    ia, ia_hash);
-	IN_IFADDR_WUNLOCK();
-
-	if (vhid > 0) {
-		if (carp_attach_p != NULL)
-			error = (*carp_attach_p)(&ia->ia_ifa, vhid);
-		else
-			error = EPROTONOSUPPORT;
-	}
-	if (error)
-		return (error);
-
-	/*
-	 * Give the interface a chance to initialize
-	 * if this is its first address,
-	 * and to validate the address if necessary.
-	 */
-	if (ifp->if_ioctl != NULL &&
-	    (error = (*ifp->if_ioctl)(ifp, SIOCSIFADDR, (caddr_t)ia)) != 0)
-			/* LIST_REMOVE(ia, ia_hash) is done in in_control */
-			return (error);
-
-	/*
-	 * Be compatible with network classes, if netmask isn't supplied,
-	 * guess it based on classes.
-	 */
-	if (!masksupplied) {
-		if (IN_CLASSA(i))
-			ia->ia_subnetmask = IN_CLASSA_NET;
-		else if (IN_CLASSB(i))
-			ia->ia_subnetmask = IN_CLASSB_NET;
-		else
-			ia->ia_subnetmask = IN_CLASSC_NET;
-		ia->ia_sockmask.sin_addr.s_addr = htonl(ia->ia_subnetmask);
-	}
-	ia->ia_subnet = i & ia->ia_subnetmask;
-	in_socktrim(&ia->ia_sockmask);
-
-	/*
-	 * Add route for the network.
-	 */
-	flags = RTF_UP;
-	ia->ia_ifa.ifa_metric = ifp->if_metric;
-	if (ifp->if_flags & IFF_BROADCAST) {
-		if (ia->ia_subnetmask == IN_RFC3021_MASK)
-			ia->ia_broadaddr.sin_addr.s_addr = INADDR_BROADCAST;
-		else
-			ia->ia_broadaddr.sin_addr.s_addr =
-			    htonl(ia->ia_subnet | ~ia->ia_subnetmask);
-	} else if (ifp->if_flags & IFF_LOOPBACK) {
-		ia->ia_dstaddr = ia->ia_addr;
-		flags |= RTF_HOST;
-	} else if (ifp->if_flags & IFF_POINTOPOINT) {
-		if (ia->ia_dstaddr.sin_family != AF_INET)
-			return (0);
-		flags |= RTF_HOST;
-	}
-	if (!vhid && (error = in_addprefix(ia, flags)) != 0)
-		return (error);
-
-	if (ia->ia_addr.sin_addr.s_addr == INADDR_ANY)
-		return (0);
-
-	if (ifp->if_flags & IFF_POINTOPOINT &&
-	    ia->ia_dstaddr.sin_addr.s_addr == ia->ia_addr.sin_addr.s_addr)
-			return (0);
-
-	/*
-	 * add a loopback route to self
-	 */
-	if (V_useloopback && !vhid && !(ifp->if_flags & IFF_LOOPBACK)) {
-		struct route ia_ro;
-
-		bzero(&ia_ro, sizeof(ia_ro));
-		*((struct sockaddr_in *)(&ia_ro.ro_dst)) = ia->ia_addr;
-		rtalloc_ign_fib(&ia_ro, 0, RT_DEFAULT_FIB);
-		if ((ia_ro.ro_rt != NULL) && (ia_ro.ro_rt->rt_ifp != NULL) &&
-		    (ia_ro.ro_rt->rt_ifp == V_loif)) {
-			RT_LOCK(ia_ro.ro_rt);
-			RT_ADDREF(ia_ro.ro_rt);
-			RTFREE_LOCKED(ia_ro.ro_rt);
-		} else
-			error = ifa_add_loopback_route((struct ifaddr *)ia,
-			    (struct sockaddr *)&ia->ia_addr);
-		if (error == 0)
-			ia->ia_flags |= IFA_RTSELF;
-		if (ia_ro.ro_rt != NULL)
-			RTFREE(ia_ro.ro_rt);
-	}
-
-	return (error);
-}
-
 #define rtinitflags(x) \
 	((((x)->ia_ifp->if_flags & (IFF_LOOPBACK | IFF_POINTOPOINT)) != 0) \
 	    ? RTF_HOST : 0)
@@ -1007,44 +949,27 @@ in_scrubprefix(struct in_ifaddr *target, u_int fla
 
 	/*
 	 * Remove the loopback route to the interface address.
-	 * The "useloopback" setting is not consulted because if the
-	 * user configures an interface address, turns off this
-	 * setting, and then tries to delete that interface address,
-	 * checking the current setting of "useloopback" would leave
-	 * that interface address loopback route untouched, which
-	 * would be wrong. Therefore the interface address loopback route
-	 * deletion is unconditional.
 	 */
 	if ((target->ia_addr.sin_addr.s_addr != INADDR_ANY) &&
 	    !(target->ia_ifp->if_flags & IFF_LOOPBACK) &&
-	    (target->ia_flags & IFA_RTSELF)) {
-		struct route ia_ro;
-		int freeit = 0;
+	    (flags & LLE_STATIC)) {
+		struct in_ifaddr *eia;
 
-		bzero(&ia_ro, sizeof(ia_ro));
-		*((struct sockaddr_in *)(&ia_ro.ro_dst)) = target->ia_addr;
-		rtalloc_ign_fib(&ia_ro, 0, 0);
-		if ((ia_ro.ro_rt != NULL) && (ia_ro.ro_rt->rt_ifp != NULL) &&
-		    (ia_ro.ro_rt->rt_ifp == V_loif)) {
-			RT_LOCK(ia_ro.ro_rt);
-			if (ia_ro.ro_rt->rt_refcnt <= 1)
-				freeit = 1;
-			else if (flags & LLE_STATIC) {
-				RT_REMREF(ia_ro.ro_rt);
-				target->ia_flags &= ~IFA_RTSELF;
-			}
-			RTFREE_LOCKED(ia_ro.ro_rt);
-		}
-		if (freeit && (flags & LLE_STATIC)) {
+		eia = more_localip(target);
+
+		if (eia != NULL) {
+			error = ifa_switch_loopback_route((struct ifaddr *)eia,
+			    (struct sockaddr *)&target->ia_addr);
+			ifa_free(&eia->ia_ifa);
+		} else {
 			error = ifa_del_loopback_route((struct ifaddr *)target,
 			    (struct sockaddr *)&target->ia_addr);
-			if (error == 0)
-				target->ia_flags &= ~IFA_RTSELF;
 		}
-		if ((flags & LLE_STATIC) &&
-			!(target->ia_ifp->if_flags & IFF_NOARP))
+
+		if (!(target->ia_ifp->if_flags & IFF_NOARP))
 			/* remove arp cache */
-			arp_ifscrub(target->ia_ifp, IA_SIN(target)->sin_addr.s_addr);
+			arp_ifscrub(target->ia_ifp,
+			    IA_SIN(target)->sin_addr.s_addr);
 	}
 
 	if (rtinitflags(target)) {
Index: sys/netinet/raw_ip.c
===================================================================
--- sys/netinet/raw_ip.c	(revision 257503)
+++ sys/netinet/raw_ip.c	(working copy)
@@ -774,8 +774,6 @@ rip_ctlinput(int cmd, struct sockaddr *sa, void *v
 			flags |= RTF_HOST;
 
 		err = ifa_del_loopback_route((struct ifaddr *)ia, sa);
-		if (err == 0)
-			ia->ia_flags &= ~IFA_RTSELF;
 
 		err = rtinit(&ia->ia_ifa, RTM_ADD, flags);
 		if (err == 0)
@@ -782,8 +780,6 @@ rip_ctlinput(int cmd, struct sockaddr *sa, void *v
 			ia->ia_flags |= IFA_ROUTE;
 
 		err = ifa_add_loopback_route((struct ifaddr *)ia, sa);
-		if (err == 0)
-			ia->ia_flags |= IFA_RTSELF;
 
 		ifa_free(&ia->ia_ifa);
 		break;

--SvF6CGw9fzJC4Rcx--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131101124720.GF52889>