Package: sysvinit
Version: 2.88dsf-13
Severity: normal
Tags: patch
User: [email protected]
Usertags: kfreebsd

Hi!

On BSDs (and particularly on GNU/kFreeBSD) the halt code to shut down
network interfaces is buggy. The problem stems from the fact that
SIOCGIFCONF has some portability issues.

On Linux the buffer returned is a list of consecutive ifreq structs,
each of the same size. On the BSDs the ifreq structs can have
different sizes, depending on the sockaddr length, but always with at
least a size of ifreq. So the next ifreq struct has to be computed
dynamically. I'm using _SIZEOF_ADDR_IFREQ because at least the BSDs
provide that macro, so it might avoid the need to define it at all.

Because the package does not have any kind of configure support I've
hardcoded a list of systems that have the sa_len member in struct
sockaddr, those same that have variable ifreq structs returned from
SIOCGIFCONF.

The patch also fixes few more things:

* The loopback interface on BSDs can be named as something like lo0,
  so check only the first two characters.
* On the BSDs the ifr_flags is split into two shorts, and to properly
  change its value, it needs to be composed from both ifr_flags and
  ifr_flagshigh, we use a temporary for that now.

This shows up on GNU/kFreeBSD as extremely ugly errors on halt/reboot as
the interface name is garbage, the SIOCGIFFLAGS fails and then prints
that garbage string stating that it's not a known device or file. It's
also confusing the fact that the error messages are prefixed with
"ifdown" which might incorrectly point people to ifupdown. This might
be worth changing too, though.

Patch attached.

thanks,
guillem
---
 src/ifdown.c |   72 +++++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 51 insertions(+), 21 deletions(-)

--- a/src/ifdown.c
+++ b/src/ifdown.c
@@ -37,6 +37,23 @@ char *v_ifdown = "@(#)ifdown.c  1.11  02
 
 #define MAX_IFS	64
 
+/* XXX: Ideally this would get detected at configure time... */
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || \
+    defined(__NetBSD__) || defined(__OpenBSD__)
+#define HAVE_SOCKADDR_SA_LEN 1
+#endif
+
+#ifndef _SIZEOF_ADDR_IFREQ
+#ifdef HAVE_SOCKADDR_SA_LEN
+#define _SIZEOF_ADDR_IFREQ(ifr) \
+	((ifr).ifr_addr.sa_len > sizeof(struct sockaddr) ? \
+	 (sizeof((ifr).ifr_name) + (ifr).ifr_addr.sa_len) : \
+	  sizeof(struct ifreq))
+#else
+#define _SIZEOF_ADDR_IFREQ(ifr) sizeof(struct ifreq)
+#endif
+#endif
+
 /*
  *	First, we find all shaper devices and down them. Then we
  *	down all real interfaces. This is because the comment in the
@@ -45,10 +62,10 @@ char *v_ifdown = "@(#)ifdown.c  1.11  02
  */
 int ifdown(void)
 {
-	struct ifreq ifr[MAX_IFS];
+	char ifr_buf[sizeof(struct ifreq) * MAX_IFS];
+	char *ifr_end;
 	struct ifconf ifc;
-	int i, fd;
-	int numif;
+	int fd;
 	int shaper;
 
 	if ((fd = socket(AF_INET, SOCK_DGRAM, 0)) < 0) {
@@ -56,8 +73,8 @@ int ifdown(void)
 		perror("socket");
 		return -1;
 	}
-	ifc.ifc_len = sizeof(ifr);
-	ifc.ifc_req = ifr;
+	ifc.ifc_len = sizeof(ifr_buf);
+	ifc.ifc_buf = ifr_buf;
 
 	if (ioctl(fd, SIOCGIFCONF, &ifc) < 0) {
 		fprintf(stderr, "ifdown: ");
@@ -65,42 +82,55 @@ int ifdown(void)
 		close(fd);
 		return -1;
 	}
-	numif = ifc.ifc_len / sizeof(struct ifreq);
+	ifr_end = ifr_buf + ifc.ifc_len;
 
 	for (shaper = 1; shaper >= 0; shaper--) {
-		for (i = 0; i < numif; i++) {
+		char *ifr_next = ifr_buf;
+
+		while (ifr_next < ifr_end) {
+			struct ifreq *ifr;
+			int flags;
 
-			if ((strncmp(ifr[i].ifr_name, "shaper", 6) == 0)
+			ifr = (struct ifreq *)ifr_next;
+			ifr_next += _SIZEOF_ADDR_IFREQ(*ifr);
+
+			if ((strncmp(ifr->ifr_name, "shaper", 6) == 0)
 			    != shaper) continue;
 
-			if (strcmp(ifr[i].ifr_name, "lo") == 0)
+			if (strncmp(ifr->ifr_name, "lo", 2) == 0)
 				continue;
-			if (strchr(ifr[i].ifr_name, ':') != NULL)
+			if (strchr(ifr->ifr_name, ':') != NULL)
 				continue;
 
 			/* Read interface flags */
-			if (ioctl(fd, SIOCGIFFLAGS, &ifr[i]) < 0) {
+			if (ioctl(fd, SIOCGIFFLAGS, ifr) < 0) {
 				fprintf(stderr, "ifdown: shutdown ");
-				perror(ifr[i].ifr_name);
+				perror(ifr->ifr_name);
 				continue;
 			}
 			/*
 			 * Expected in <net/if.h> according to
 			 * "UNIX Network Programming".
 			 */
-#ifdef ifr_flags
-# define IRFFLAGS	ifr_flags
-#else	/* Present on kFreeBSD */
-# define IRFFLAGS	ifr_flagshigh
+#ifdef ifr_flagshigh
+			flags = (ifr->ifr_flags & 0xffff) |
+			        (ifr->ifr_flagshigh << 16);
+#else
+			flags = ifr->ifr_flags;
+#endif
+			if (flags & IFF_UP) {
+				flags &= ~(IFF_UP);
+#ifdef ifr_flagshigh
+				ifr->ifr_flags = flags & 0xffff;
+				ifr->ifr_flagshigh = flags >> 16;
+#else
+				ifr->ifr_flags = flags;
 #endif
-			if (ifr[i].IRFFLAGS & IFF_UP) {
-				ifr[i].IRFFLAGS &= ~(IFF_UP);
-				if (ioctl(fd, SIOCSIFFLAGS, &ifr[i]) < 0) {
+				if (ioctl(fd, SIOCSIFFLAGS, ifr) < 0) {
 					fprintf(stderr, "ifdown: shutdown ");
-					perror(ifr[i].ifr_name);
+					perror(ifr->ifr_name);
 				}
 			}
-#undef IRFFLAGS
 		}
 	}
 	close(fd);
_______________________________________________
Pkg-sysvinit-devel mailing list
[email protected]
http://lists.alioth.debian.org/mailman/listinfo/pkg-sysvinit-devel

Reply via email to