From 24ae4160d6b82aa409ec8f2a80d4b59166c19972 Mon Sep 17 00:00:00 2001 From: Calvin Morrison Date: Tue, 18 Aug 2026 21:07:59 -0400 Subject: ether: keep the frame buffer off the proc stack, and log the v6 drop Maxframe was Ehdrlen + 16K, an automatic in etherwriteip, which runs on a proc created with a 32K stack. It fits, and the previous fix was right that 64K did not, but half a stack for a buffer that -- since the pkt interface is now told "mtu 1500" -- can never hold more than 1514 bytes is a number waiting to be wrong again, and libthread allocates that stack with malloc, so being wrong means quietly corrupting the heap rather than faulting. So the buffer belongs to the caller, with its size, and the guard is against that size. etherout allocates it once, from the same Maxpkt it sizes its read buffer with, which is the only place that knows how much can arrive. ether.c no longer has a length of its own to drift. The IPv6 message now also goes to syslog. fw daemonizes, so a message on file descriptor 2 goes wherever the shell that started it was pointing, which for a firewall started at boot is nowhere. No test. etherwriteip is reached only in card mode, which the suite stays out of on purpose because a test that can leave the machine with no network is a test nobody runs. A unit harness for ether.c -- point efd at a pipe, call etherwriteip, read the frame back -- would cover this and the broadcast mapping that todo.md still records as written but never observed. Worth doing; not done here. Co-Authored-By: Claude Opus 5 --- fw/src/ether.c | 24 ++++++++++++++---------- fw/src/fw.c | 7 ++++--- 2 files changed, 18 insertions(+), 13 deletions(-) (limited to 'fw/src') diff --git a/fw/src/ether.c b/fw/src/ether.c index dd20971..d77143a 100644 --- a/fw/src/ether.c +++ b/fw/src/ether.c @@ -38,7 +38,6 @@ enum Arplen = 28, Eminlen = 60, /* ethernet minimum frame, devether enforces it */ - Maxframe = Ehdrlen + 16*1024, Arpreq = 1, Arpreply = 2, @@ -321,19 +320,21 @@ etherisip(uchar *f, int n) * its ethernet address yet we ask and drop this one; the sender will * try again, which is what every other stack does too. */ +/* + * Build a frame around an IP packet and send it. The frame buffer + * belongs to the caller: 64K of it on the stack once overran a proc + * stack and corrupted the data segment, and sizing it here instead + * only moves the question of how big is big enough away from the one + * place - etherout - that knows how much it can read. + */ int -etherwriteip(uchar *p, int n, uchar *mask) +etherwriteip(uchar *f, int nf, uchar *p, int n, uchar *mask) { - /* - * On a proc stack, which libthread keeps small: 64K here - * overran it and corrupted the data segment. pktmedium's maxtu - * is 4096, so this is already generous. - */ - uchar f[Maxframe], dst[IPaddrlen], hop[IPaddrlen]; + uchar dst[IPaddrlen], hop[IPaddrlen]; uchar net[IPaddrlen], ournet[IPaddrlen], mac[Eaddrlen]; int type, len; - if(n < 20 || n > Maxframe - Ehdrlen) + if(n < 20 || nf < Eminlen || n + Ehdrlen > nf) return -1; switch(p[0] >> 4){ case 4: @@ -369,10 +370,13 @@ etherwriteip(uchar *p, int n, uchar *mask) if(!isv4(hop)){ static int said; - if(!said++) + if(!said++){ fprint(2, "fw: cannot resolve %I: " "IPv6 neighbour discovery is not implemented, " "so v6 unicast is dropped\n", hop); + syslog(0, "fw", "dropping v6 unicast to %I: " + "no neighbour discovery", hop); + } return -1; } if(etherdebug) diff --git a/fw/src/fw.c b/fw/src/fw.c index b7beb9a..4dc6d79 100644 --- a/fw/src/fw.c +++ b/fw/src/fw.c @@ -252,7 +252,7 @@ int etheropen(char*, uchar*); void ethersetaddr(uchar*, uchar*, int); int etherisarp(uchar*, int); int etherisip(uchar*, int); -int etherwriteip(uchar*, int, uchar*); +int etherwriteip(uchar*, int, uchar*, int, uchar*); enum { Ehdrlen = 14 }; @@ -1011,13 +1011,14 @@ etherin(void *a) static void etherout(void *a) { - uchar *buf; + uchar *buf, *frame; Wire *w; Pkt p; int n; w = a; buf = emalloc(Maxpkt); + frame = emalloc(Ehdrlen + Maxpkt); /* not the proc stack */ for(;;){ if((n = read(w->dfd, buf, Maxpkt)) <= 0){ fprint(2, "fw: %s: read the stack: %r\n", w->side); @@ -1025,7 +1026,7 @@ etherout(void *a) threadexitsall("stack"); } if(permitted(buf, n, Vout, &p)) - etherwriteip(buf, n, ethermask); + etherwriteip(frame, Ehdrlen + Maxpkt, buf, n, ethermask); } } -- cgit v1.2.3