I have fixed the problems in the SIOCGIFCONF implementation, so that
racoon in current works again.  The basic issue was that
sys/net/if.c:ifconf() was doing the wrong length calculation.  I've
documented the expected behavior of the function, written down
invariants, and also fixed some subtle bugs where the space variable was
decremented even if there was not adequate space.  Because of
sockaddr_storage, the hairy case where ifreq is expanded due to
overlength sockaddrs can't occur, so I replaced the complex code with a
KASSERT.  I also added KASSERTs to check a bunch of things; clearly this
code is too tricky.  Because of that, I split out the handling of the
cases where data is returned vs getting sizes more explicitly, going for
readability and being clearly correct rather than compact.

We indeed need a regression test.  Does anyone want to write a program
that calls getifaddrs and also does SIOCGIFCONF and compares the
results?

Here's the new code, and then the diff.  I will commit this in a week or
so if there are no objections.

/*
 * Return interface configuration
 * of system.  List may be used
 * in later ioctl's (above) to get
 * other information.
 *
 * Each record is a struct ifreq.  Before the addition of
 * sockaddr_storage, the API rule was that sockaddr flavors that did
 * not fit would extend beyond the struct ifreq, with the next struct
 * ifreq starting sa_len beyond the struct sockaddr.  Because the
 * union in struct ifreq includes struct sockaddr_storage, every kind
 * of sockaddr must fit.  Thus, there are no longer any overlength
 * records.
 *
 * Records are added to the user buffer if they fit, and ifc_len is
 * adjusted to the length that was written.  Thus, the user is only
 * assured of getting the complete list if ifc_len on return is at
 * least sizeof(struct ifreq) less than it was on entry.
 *
 * If the user buffer pointer is NULL, this routine copies no data and
 * returns the amount of space that would be needed.
 *
 * Invariants:
 * ifrp points to the next part of the user's buffer to be used.  If
 * ifrp != NULL, space holds the number of bytes remaining that we may
 * write at ifrp.  Otherwise, space holds the number of bytes that
 * would have been written had there been adequate space.
 */
/*ARGSUSED*/
int
ifconf(u_long cmd, void *data)
{
	struct ifconf *ifc = (struct ifconf *)data;
	struct ifnet *ifp;
	struct ifaddr *ifa;
	struct ifreq ifr, *ifrp;
	int space, error = 0;
	const int sz = (int)sizeof(struct ifreq);

	if ((ifrp = ifc->ifc_req) == NULL)
		space = 0;
	else
		space = ifc->ifc_len;
	IFNET_FOREACH(ifp) {
		(void)strncpy(ifr.ifr_name, ifp->if_xname,
		    sizeof(ifr.ifr_name));
		if (ifr.ifr_name[sizeof(ifr.ifr_name) - 1] != '\0')
			return ENAMETOOLONG;
		if (TAILQ_EMPTY(&ifp->if_addrlist)) {
			/* Interface with no addresses - send zero sockaddr. */
			memset(&ifr.ifr_addr, 0, sizeof(ifr.ifr_addr));
			if (ifrp != NULL)
			{
				if (space >= sz) {
					error = copyout(&ifr, ifrp, sz);
					if (error != 0)
						return (error);
					ifrp++; space -= sz;
				}
			}
			else
				space += sz;
			continue;
		}

		TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
			struct sockaddr *sa = ifa->ifa_addr;
			/* all sockaddrs must fit in sockaddr_storage */
			KASSERT(sa->sa_len <= sizeof(ifr.ifr_ifru));

			if (ifrp != NULL)
			{
				ifr.ifr_addr = *sa;
				if (space >= sz) {
					error = copyout(&ifr, ifrp, sz);
					if (error != 0)
						return (error);
					ifrp++; space -= sz;
				}
			}
			else
				space += sz;
		}
	}
	if (ifrp != NULL)
	{
		KASSERT(0 <= space && space <= ifc->ifc_len);
		ifc->ifc_len -= space;
	}
	else
	{
		KASSERT(space >= 0);
		ifc->ifc_len = space;
	}
	return (0);
}

--- if.c	31 Aug 2007 08:23:43 -0400	1.197
+++ if.c	31 Aug 2007 09:31:27 -0400	
@@ -1602,6 +1602,28 @@ ifioctl(struct socket *so, u_long cmd, v
  * of system.  List may be used
  * in later ioctl's (above) to get
  * other information.
+ *
+ * Each record is a struct ifreq.  Before the addition of
+ * sockaddr_storage, the API rule was that sockaddr flavors that did
+ * not fit would extend beyond the struct ifreq, with the next struct
+ * ifreq starting sa_len beyond the struct sockaddr.  Because the
+ * union in struct ifreq includes struct sockaddr_storage, every kind
+ * of sockaddr must fit.  Thus, there are no longer any overlength
+ * records.
+ *
+ * Records are added to the user buffer if they fit, and ifc_len is
+ * adjusted to the length that was written.  Thus, the user is only
+ * assured of getting the complete list if ifc_len on return is at
+ * least sizeof(struct ifreq) less than it was on entry.
+ *
+ * If the user buffer pointer is NULL, this routine copies no data and
+ * returns the amount of space that would be needed.
+ *
+ * Invariants:
+ * ifrp points to the next part of the user's buffer to be used.  If
+ * ifrp != NULL, space holds the number of bytes remaining that we may
+ * write at ifrp.  Otherwise, space holds the number of bytes that
+ * would have been written had there been adequate space.
  */
 /*ARGSUSED*/
 int
@@ -1612,8 +1634,7 @@ ifconf(u_long cmd, void *data)
 	struct ifaddr *ifa;
 	struct ifreq ifr, *ifrp;
 	int space, error = 0;
-	const int sz = offsetof(struct ifreq, ifr_ifru) +
-	    sizeof(struct sockaddr);
+	const int sz = (int)sizeof(struct ifreq);
 
 	if ((ifrp = ifc->ifc_req) == NULL)
 		space = 0;
@@ -1625,46 +1646,51 @@ ifconf(u_long cmd, void *data)
 		if (ifr.ifr_name[sizeof(ifr.ifr_name) - 1] != '\0')
 			return ENAMETOOLONG;
 		if (TAILQ_EMPTY(&ifp->if_addrlist)) {
+			/* Interface with no addresses - send zero sockaddr. */
 			memset(&ifr.ifr_addr, 0, sizeof(ifr.ifr_addr));
-			if (space >= sz) {
-				error = copyout(&ifr, ifrp, sz);
-				if (error != 0)
-					return (error);
-				ifrp++;
+			if (ifrp != NULL)
+			{
+				if (space >= sz) {
+					error = copyout(&ifr, ifrp, sz);
+					if (error != 0)
+						return (error);
+					ifrp++; space -= sz;
+				}
 			}
-			space -= sizeof(struct ifreq);
+			else
+				space += sz;
 			continue;
 		}
 
 		TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
 			struct sockaddr *sa = ifa->ifa_addr;
-			if (sa->sa_len <= sizeof(*sa)) {
+			/* all sockaddrs must fit in sockaddr_storage */
+			KASSERT(sa->sa_len <= sizeof(ifr.ifr_ifru));
+
+			if (ifrp != NULL)
+			{
 				ifr.ifr_addr = *sa;
 				if (space >= sz) {
 					error = copyout(&ifr, ifrp, sz);
-					ifrp++;
+					if (error != 0)
+						return (error);
+					ifrp++; space -= sz;
 				}
-				space -= sizeof(struct ifreq);
-			} else {
-				space -= sa->sa_len - sizeof(*sa) + sz;
-				if (space < 0)
-					continue;
-				error = copyout(&ifr, ifrp,
-				    sizeof(ifr.ifr_name));
-				if (error == 0)
-					error = copyout(sa,
-					    &ifrp->ifr_addr, sa->sa_len);
-				ifrp = (struct ifreq *)
-				    (sa->sa_len + (char *)&ifrp->ifr_addr);
 			}
-			if (error != 0)
-				return (error);
+			else
+				space += sz;
 		}
 	}
 	if (ifrp != NULL)
+	{
+		KASSERT(0 <= space && space <= ifc->ifc_len);
 		ifc->ifc_len -= space;
+	}
 	else
-		ifc->ifc_len = -space;
+	{
+		KASSERT(space >= 0);
+		ifc->ifc_len = space;
+	}
 	return (0);
 }