svn commit: r227791 - head/sys/netinet
Gleb Smirnoff
glebius at FreeBSD.org
Mon Nov 21 14:10:13 UTC 2011
Author: glebius
Date: Mon Nov 21 14:10:13 2011
New Revision: 227791
URL: http://svn.freebsd.org/changeset/base/227791
Log:
Historically in_control() did not check sockaddrs supplied with
structs ifreq/in_aliasreq and there've been several panics due
to that problem. All these panics were fixed just a couple of
lines above the panicing code.
Take a more general approach: sanity check sockaddrs supplied
with SIOCAIFADDR and SIOCSIF*ADDR at the beggining of the
function and drop all checks below.
One check is now disabled due to strange code in ifconfig(8)
that I've removed recently. I'm going to enable it with next
__FreeBSD_version bump.
Historically in_ifinit() was able to recover from an error
and restore old address. Nowadays this feature isn't working
for all error cases, but for some of them. I suppose no software
relies on this behavior, so I'd like to remove it, since this
simplifies code a lot.
Also, move if_scrub() earlier in the in_ifinit(). It is more
correct to wipe routes before removing address from local
address list, and interface address list.
Silence from: bz, brooks, andre, rwatson, 3 weeks
Modified:
head/sys/netinet/in.c
Modified: head/sys/netinet/in.c
==============================================================================
--- head/sys/netinet/in.c Mon Nov 21 13:40:35 2011 (r227790)
+++ head/sys/netinet/in.c Mon Nov 21 14:10:13 2011 (r227791)
@@ -234,16 +234,42 @@ in_control(struct socket *so, u_long cmd
* in_lifaddr_ioctl() and ifp->if_ioctl().
*/
switch (cmd) {
- case SIOCAIFADDR:
- case SIOCDIFADDR:
case SIOCGIFADDR:
case SIOCGIFBRDADDR:
case SIOCGIFDSTADDR:
case SIOCGIFNETMASK:
+ case SIOCDIFADDR:
+ break;
+ 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) historically doesn't set af_family for mask
+ * for unknown reason.
+ */
+ 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;
case SIOCSIFADDR:
case SIOCSIFBRDADDR:
case SIOCSIFDSTADDR:
case SIOCSIFNETMASK:
+ if (ifr->ifr_addr.sa_family != AF_INET ||
+ ifr->ifr_addr.sa_len != sizeof(struct sockaddr_in))
+ return (EINVAL);
break;
case SIOCALIFADDR:
@@ -349,7 +375,7 @@ in_control(struct socket *so, u_long cmd
switch (cmd) {
case SIOCAIFADDR:
case SIOCDIFADDR:
- if (ifra->ifra_addr.sin_family == AF_INET) {
+ {
struct in_ifaddr *oia;
IN_IFADDR_RLOCK();
@@ -506,7 +532,7 @@ in_control(struct socket *so, u_long cmd
goto out;
case SIOCSIFNETMASK:
- ia->ia_sockmask.sin_addr = ifra->ifra_addr.sin_addr;
+ ia->ia_sockmask = *(struct sockaddr_in *)&ifr->ifr_addr;
ia->ia_subnetmask = ntohl(ia->ia_sockmask.sin_addr.s_addr);
goto out;
@@ -514,14 +540,12 @@ in_control(struct socket *so, u_long cmd
maskIsNew = 0;
hostIsNew = 1;
error = 0;
- if (ia->ia_addr.sin_family == AF_INET) {
- if (ifra->ifra_addr.sin_len == 0) {
- ifra->ifra_addr = ia->ia_addr;
- hostIsNew = 0;
- } else if (ifra->ifra_addr.sin_addr.s_addr ==
- ia->ia_addr.sin_addr.s_addr)
- hostIsNew = 0;
- }
+ if (ifra->ifra_addr.sin_len == 0) {
+ ifra->ifra_addr = ia->ia_addr;
+ hostIsNew = 0;
+ } else 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
@@ -552,7 +576,7 @@ in_control(struct socket *so, u_long cmd
break;
if ((ifp->if_flags & IFF_BROADCAST) &&
- (ifra->ifra_broadaddr.sin_family == AF_INET))
+ ifra->ifra_broadaddr.sin_len)
ia->ia_broadaddr = ifra->ifra_broadaddr;
if (error == 0) {
ii = ((struct in_ifinfo *)ifp->if_afdata[AF_INET]);
@@ -608,31 +632,26 @@ in_control(struct socket *so, u_long cmd
IN_IFADDR_WLOCK();
TAILQ_REMOVE(&V_in_ifaddrhead, ia, ia_link);
- if (ia->ia_addr.sin_family == AF_INET) {
- struct in_ifaddr *if_ia;
- LIST_REMOVE(ia, ia_hash);
- IN_IFADDR_WUNLOCK();
- /*
- * If this is the last IPv4 address configured on this
- * interface, leave the all-hosts group.
- * No state-change report need be transmitted.
- */
- if_ia = NULL;
- IFP_TO_IA(ifp, if_ia);
- if (if_ia == NULL) {
- ii = ((struct in_ifinfo *)ifp->if_afdata[AF_INET]);
- IN_MULTI_LOCK();
- if (ii->ii_allhosts) {
- (void)in_leavegroup_locked(ii->ii_allhosts,
- NULL);
- ii->ii_allhosts = NULL;
- }
- IN_MULTI_UNLOCK();
- } else
- ifa_free(&if_ia->ia_ifa);
+ LIST_REMOVE(ia, ia_hash);
+ IN_IFADDR_WUNLOCK();
+ /*
+ * 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) {
+ ii = ((struct in_ifinfo *)ifp->if_afdata[AF_INET]);
+ IN_MULTI_LOCK();
+ if (ii->ii_allhosts) {
+ (void)in_leavegroup_locked(ii->ii_allhosts, NULL);
+ ii->ii_allhosts = NULL;
+ }
+ IN_MULTI_UNLOCK();
} else
- IN_IFADDR_WUNLOCK();
+ ifa_free(&iap->ia_ifa);
+
ifa_free(&ia->ia_ifa); /* in_ifaddrhead */
out:
if (ia != NULL)
@@ -828,50 +847,29 @@ in_ifinit(struct ifnet *ifp, struct in_i
int scrub)
{
register u_long i = ntohl(sin->sin_addr.s_addr);
- struct sockaddr_in oldaddr;
int flags = RTF_UP, error = 0;
- oldaddr = ia->ia_addr;
- if (oldaddr.sin_family == AF_INET)
+ if (scrub)
+ in_scrubprefix(ia, LLE_STATIC);
+
+ IN_IFADDR_WLOCK();
+ if (ia->ia_addr.sin_family == AF_INET)
LIST_REMOVE(ia, ia_hash);
ia->ia_addr = *sin;
- if (ia->ia_addr.sin_family == AF_INET) {
- IN_IFADDR_WLOCK();
- LIST_INSERT_HEAD(INADDR_HASH(ia->ia_addr.sin_addr.s_addr),
- ia, ia_hash);
- IN_IFADDR_WUNLOCK();
- }
+ LIST_INSERT_HEAD(INADDR_HASH(ia->ia_addr.sin_addr.s_addr),
+ ia, ia_hash);
+ IN_IFADDR_WUNLOCK();
+
/*
* 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) {
+ 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 */
- ia->ia_addr = oldaddr;
- IN_IFADDR_WLOCK();
- if (ia->ia_addr.sin_family == AF_INET)
- LIST_INSERT_HEAD(INADDR_HASH(
- ia->ia_addr.sin_addr.s_addr), ia, ia_hash);
- else
- /*
- * If oldaddr family is not AF_INET (e.g.
- * interface has been just created) in_control
- * does not call LIST_REMOVE, and we end up
- * with bogus ia entries in hash
- */
- LIST_REMOVE(ia, ia_hash);
- IN_IFADDR_WUNLOCK();
return (error);
- }
- }
- if (scrub) {
- ia->ia_ifa.ifa_addr = (struct sockaddr *)&oldaddr;
- in_ifscrub(ifp, ia, LLE_STATIC);
- ia->ia_ifa.ifa_addr = (struct sockaddr *)&ia->ia_addr;
- }
+
/*
* Be compatible with network classes, if netmask isn't supplied,
* guess it based on classes.
@@ -916,11 +914,9 @@ in_ifinit(struct ifnet *ifp, struct in_i
if (ia->ia_addr.sin_addr.s_addr == INADDR_ANY)
return (0);
- if (ifp->if_flags & IFF_POINTOPOINT) {
- if (ia->ia_dstaddr.sin_addr.s_addr == ia->ia_addr.sin_addr.s_addr)
+ 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
More information about the svn-src-head
mailing list