fw: read the interface tables a line at a time, and test card mode at all

learnaddr, reclaim and takecard each read a status or route file into a
fixed 1024-byte buffer and split it into at most eight lines.  I called
this a defect that could lose the default route.  It could not: routes
come out sorted, and 0.0.0.0 sorts first, so the default route is on
the first line of the table and both limits are reached long after it
has been found.  The report was wrong about the consequence.

The limits are still worth removing, and one thing in there was a real
mistake: learnaddr and takecard looked for the device name anywhere in
the status text, addresses included, rather than in the field that
holds it.  An interface whose address contained the name of the device
being looked for would have matched.  Contrived, but there is no reason
to be searching a blob for something that has a place of its own.  Bio
reads line by line, the device is matched against the device field, and
nothing has a length limit any more.

More to the point, none of this had ever been run.  Card mode is
outside the suite because taking the machine's card away is how you
end up with no network -- but a second card that nothing is using can
be taken safely.  The suite now binds #l1 into /net, puts it in an IP
stack of its own with an address and a default route, and hands it to
fw with no -a and no -g:

	fw: /net/ether1 has 10.9.9.1/120, gateway 10.9.9.254
	fw: took /net/ether1 away from .../ipifc/0
	fw: protected .../ipifc/0 addr 10.9.9.1 /120
	fw: default route via 10.9.9.254

Six checks on that: the address and gateway are read off the interface
rather than repeated on the command line, the card is taken, what
replaces it is a pkt interface at the card's mtu holding the same
address, and the route the card carried is put back.  Skipped when
there is no spare card, which is why it says so rather than passing
quietly.  72 pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

2 files changed, 147 insertions, 59 deletions

diff --git a/fw/src/fw.c b/fw/src/fw.c
index 1fb88bd..8901c2a 100644
--- a/fw/src/fw.c
+++ b/fw/src/fw.c
@@ -1126,6 +1126,36 @@ etherout(void *a)
 }
 
 /*
+ * The first n lines of a Biobuf, copied out, since Brdline's buffer is
+ * only good until the next one.  Returns how many there were.
+ */
+static int
+rdline(Biobuf *b, char **lines, int n)
+{
+	char *p;
+	int i;
+
+	for(i = 0; i < n; i++){
+		if((p = Brdline(b, '\n')) == nil)
+			break;
+		p[Blinelen(b)-1] = '\0';
+		lines[i] = estrdup(p);
+	}
+	return i;
+}
+
+static void
+freelines(char **lines, int n)
+{
+	int i;
+
+	for(i = 0; i < n; i++){
+		free(lines[i]);
+		lines[i] = nil;
+	}
+}
+
+/*
  * The address, mask and gateway the stack is already using.  Asking the
  * administrator to repeat what ipconfig(8) was told is a way of getting
  * the two out of step, so read them instead: the interface status names
@@ -1134,52 +1164,53 @@ etherout(void *a)
 static int
 learnaddr(char *net, char *dev, char *addr, int naddr, char *gw, int ngw)
 {
-	char path[128], buf[1024], *lines[8], *f[8], *p;
-	int i, fd, n, nl, nf, found;
+	char path[128], *lines[2], *f[8], *p;
+	Biobuf *b;
+	int i, nf, found;
 
 	found = 0;
 	for(i = 0; i < 16 && !found; i++){
 		snprint(path, sizeof path, "%s/ipifc/%d/status", net, i);
-		if((fd = open(path, OREAD)) < 0)
-			continue;
-		n = read(fd, buf, sizeof buf - 1);
-		close(fd);
-		if(n <= 0)
-			continue;
-		buf[n] = '\0';
-		if((p = strstr(buf, dev)) == nil)
-			continue;
-		USED(p);
-		nl = getfields(buf, lines, nelem(lines), 0, "\n");
-		if(nl < 2)
+		if((b = Bopen(path, OREAD)) == nil)
 			continue;
-		/* the address line: address, then the mask as a prefix */
-		nf = tokenize(lines[1], f, nelem(f));
-		if(nf < 2)
-			continue;
-		snprint(addr, naddr, "%s%s", f[0], f[1]);
-		found = 1;
+		/*
+		 * The device line, then the first address line.  Read, not
+		 * searched: the whole status text used to be scanned for the
+		 * device name, which an address could satisfy, and it was
+		 * read into a fixed buffer that a few addresses would fill.
+		 */
+		if(rdline(b, lines, 2) == 2 && strstr(lines[0], dev) != nil){
+			nf = tokenize(lines[1], f, nelem(f));
+			if(nf >= 2){
+				snprint(addr, naddr, "%s%s", f[0], f[1]);
+				found = 1;
+			}
+		}
+		freelines(lines, 2);
+		Bterm(b);
 	}
 	if(!found)
 		return -1;
 
+	/*
+	 * The default route names the gateway.  A line at a time: a
+	 * routing table is as long as it is, and reading 1024 bytes of it
+	 * meant the default route could be off the end and the gateway
+	 * silently unknown.
+	 */
 	*gw = '\0';
 	snprint(path, sizeof path, "%s/iproute", net);
-	if((fd = open(path, OREAD)) >= 0){
-		n = read(fd, buf, sizeof buf - 1);
-		close(fd);
-		if(n > 0){
-			buf[n] = '\0';
-			nl = getfields(buf, lines, nelem(lines), 0, "\n");
-			for(i = 0; i < nl; i++){
-				nf = tokenize(lines[i], f, nelem(f));
-				if(nf >= 3 && strcmp(f[0], "0.0.0.0") == 0
-				&& strcmp(f[1], "/96") == 0){
-					snprint(gw, ngw, "%s", f[2]);
-					break;
-				}
+	if((b = Bopen(path, OREAD)) != nil){
+		while((p = Brdline(b, '\n')) != nil){
+			p[Blinelen(b)-1] = '\0';
+			nf = tokenize(p, f, nelem(f));
+			if(nf >= 3 && strcmp(f[0], "0.0.0.0") == 0
+			&& strcmp(f[1], "/96") == 0){
+				snprint(gw, ngw, "%s", f[2]);
+				break;
 			}
 		}
+		Bterm(b);
 	}
 	return 0;
 }
@@ -1207,27 +1238,31 @@ learnaddr(char *net, char *dev, char *addr, int naddr, char *gw, int ngw)
 static void
 reclaim(char *net, char *addr)
 {
-	char path[128], buf[1024], *lines[8], *f[8];
-	int i, fd, n, nl, nf;
+	char path[128], *lines[2], *f[8];
+	Biobuf *b;
+	int i, fd, nf;
 
 	for(i = 0; i < 16; i++){
 		snprint(path, sizeof path, "%s/ipifc/%d/status", net, i);
-		if((fd = open(path, OREAD)) < 0)
-			continue;
-		n = read(fd, buf, sizeof buf - 1);
-		close(fd);
-		if(n <= 0)
+		if((b = Bopen(path, OREAD)) == nil)
 			continue;
-		buf[n] = '\0';
-		nl = getfields(buf, lines, nelem(lines), 0, "\n");
-		if(nl < 2)
+		nf = rdline(b, lines, 2);
+		Bterm(b);
+		if(nf != 2){
+			freelines(lines, 2);
 			continue;
+		}
 		nf = tokenize(lines[0], f, nelem(f));
-		if(nf < 2 || strncmp(f[1], "pkt", 3) != 0)
+		if(nf < 2 || strncmp(f[1], "pkt", 3) != 0){
+			freelines(lines, 2);
 			continue;
+		}
 		nf = tokenize(lines[1], f, nelem(f));
-		if(nf < 1 || strcmp(f[0], addr) != 0)
+		if(nf < 1 || strcmp(f[0], addr) != 0){
+			freelines(lines, 2);
 			continue;
+		}
+		freelines(lines, 2);
 
 		snprint(path, sizeof path, "%s/ipifc/%d/ctl", net, i);
 		if((fd = open(path, OWRITE)) < 0)
@@ -1249,26 +1284,30 @@ reclaim(char *net, char *addr)
 static void
 takecard(char *net, char *dev)
 {
-	char path[128], buf[512], *p;
-	int i, fd, n, found;
+	char path[128], *lines[1], *f[8];
+	Biobuf *b;
+	int i, fd, nf, found;
 
 	found = 0;
 	for(i = 0; i < 16; i++){
 		snprint(path, sizeof path, "%s/ipifc/%d/status", net, i);
-		if((fd = open(path, OREAD)) < 0)
-			continue;
-		n = read(fd, buf, sizeof buf - 1);
-		close(fd);
-		if(n <= 0)
+		if((b = Bopen(path, OREAD)) == nil)
 			continue;
-		buf[n] = '\0';
-		if((p = strchr(buf, ' ')) == nil)
+		nf = rdline(b, lines, 1);
+		Bterm(b);
+		if(nf != 1)
 			continue;
-		*p = '\0';
-		if(strcmp(buf, "device") != 0 && strstr(buf, dev) == nil)
-			continue;
-		if(strstr(p+1, dev) == nil && strstr(buf, dev) == nil)
+		/*
+		 * "device <name> maxtu ...".  The name, not the rest of the
+		 * status: an address that happened to contain the device's
+		 * name used to match it.
+		 */
+		nf = tokenize(lines[0], f, nelem(f));
+		if(nf < 2 || strstr(f[1], dev) == nil){
+			freelines(lines, 1);
 			continue;
+		}
+		freelines(lines, 1);
 		snprint(path, sizeof path, "%s/ipifc/%d/ctl", net, i);
 		if((fd = open(path, OWRITE)) < 0)
 			continue;
diff --git a/fw/test/fwtest.rc b/fw/test/fwtest.rc
index 8a5a085..0325bfd 100755
--- a/fw/test/fwtest.rc
+++ b/fw/test/fwtest.rc
@@ -46,7 +46,7 @@ mkdir -p $mtpt
 # The stacks the packet checks build for themselves.  Fixed numbers are
 # fine because they are unbound again at the end of each block, and the
 # last check proves it.
-stacks=(20 21 22 23 24 25)
+stacks=(20 21 22 23 24 25 26)
 
 # A port of our own.  Conversations outlive the run that made them - a
 # devip Conv is never freed - so a fixed port makes one run's leftovers
@@ -502,6 +502,55 @@ echo '== a fragmented datagram crosses'
 	stopfw $nA $nB
 }
 
+echo '== a card'
+# Card mode has never been tested, because taking the machine's card
+# away is how you end up with no network.  A second card that nothing
+# is using can be taken safely: bound into a stack of our own, given an
+# address there, and handed to fw.  Everything up to the wire is then
+# real - reading the address off the interface, taking the card,
+# putting a pkt interface in its place, restoring the route.
+@{
+	rfork n
+	bind -a '#l1' /net >[2]/dev/null
+	spare=no
+	if(test -e /net/ether1)
+	if(~ `{grep -c ether1 /net/ipifc/*/status >[2]/dev/null | awk '{n += $1} END {print n+0}'} 0)
+		spare=yes
+	if(~ $spare no)
+		echo '	skip  no spare card to take'
+	if not {
+		mkdir -p $tmp/nC
+		bind -a '#I26' $tmp/nC
+		@{
+			conv=`{cat /fd/0}
+			echo -n 'bind ether /net/ether1' >[1=0]
+			echo -n 'add 10.9.9.1 255.255.255.0' >[1=0]
+		} <>[0] $tmp/nC/ipifc/clone
+		echo -n 'add 0.0.0.0 0.0.0.0 10.9.9.254' > $tmp/nC/iproute
+
+		# no -a and no -g: both come off the interface it is taking
+		# over, which is the whole point of not having to repeat them
+		$fw -n $tmp/nC -e /net/ether1 -m $mtpt $tmp/empty.ndb >[2] $tmp/carderr
+		sleep 2
+
+		r=`{grep -c '/net/ether1 has 10.9.9.1/120, gateway 10.9.9.254' $tmp/carderr}
+		check 'the address and gateway are read off the card' 1 $"r
+		r=`{grep -c 'took /net/ether1 away' $tmp/carderr}
+		check 'and the card is taken from the stack' 1 $"r
+
+		r=`{sed 1q $tmp/nC/ipifc/0/status | awk '{print $2}'}
+		check 'what the stack has now is a pkt interface' pkt0 $"r
+		r=`{sed 1q $tmp/nC/ipifc/0/status | awk '{print $4}'}
+		check 'at the mtu of the card it replaced' 1500 $"r
+		r=`{sed -n 2p $tmp/nC/ipifc/0/status | awk '{print $1}'}
+		check 'keeping the address' 10.9.9.1 $"r
+		r=`{awk '$1 == "0.0.0.0" {print $3}' $tmp/nC/iproute | sed 1q}
+		check 'and the route that went with the card' 10.9.9.254 $"r
+
+		stopfw $tmp/nC
+	}
+}
+
 echo '== fwstart'
 if(test -f $lib/fwstart)
 @{