From 39bf5b987e8ab53a74adec5a15b30f4eda85b357 Mon Sep 17 00:00:00 2001
From: cinap_lenrek <cinap_lenrek@felloff.net>
Date: Fri, 19 May 2023 23:42:54 +0000
Subject: devip: don't leak temporary buffers on error

when reading the status files from a conversation, ipread()
allocates a temporary buffer and calls the protocols
generator to fill the buffer and the calls
readstr() to handle the read request.

if the generator or readstr() erors, we must free the
temporary buffer.

also, allocate a more reasonable size for the "local"
and "remote" strings. allocating and zeroing 32K big
buffers for them is excessive.
---
 sys/src/9/ip/devip.c | 62 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 23 deletions(-)

diff --git a/sys/src/9/ip/devip.c b/sys/src/9/ip/devip.c
index 44800bdb9..032f53104 100644
--- a/sys/src/9/ip/devip.c
+++ b/sys/src/9/ip/devip.c
@@ -610,7 +610,9 @@ ipclose(Chan* c)
 
 enum
 {
-	Statelen=	32*1024,
+	Maxstring=	128,
+	Maxstate=	32*1024,
+	Maxstats=	32*1024,
 };
 
 static long
@@ -647,42 +649,54 @@ ipread(Chan *ch, void *a, long n, vlong off)
 		return netlogread(f, a, offset, n);
 	case Qctl:
 		buf = smalloc(16);
+		if(waserror()){
+			free(buf);
+			nexterror();
+		}
 		snprint(buf, 16, "%lud", CONV(ch->qid));
+	Readstr:
 		rv = readstr(offset, p, n, buf);
 		free(buf);
+		poperror();
 		return rv;
 	case Qremote:
-		buf = smalloc(Statelen);
+		buf = smalloc(Maxstring);
+		if(waserror()){
+			free(buf);
+			nexterror();
+		}
 		x = f->p[PROTO(ch->qid)];
 		c = x->conv[CONV(ch->qid)];
 		if(x->remote == nil) {
-			snprint(buf, Statelen, "%I!%d\n", c->raddr, c->rport);
+			snprint(buf, Maxstring, "%I!%d\n", c->raddr, c->rport);
 		} else {
-			(*x->remote)(c, buf, Statelen-2);
+			(*x->remote)(c, buf, Maxstring);
 		}
-		rv = readstr(offset, p, n, buf);
-		free(buf);
-		return rv;
+		goto Readstr;
 	case Qlocal:
-		buf = smalloc(Statelen);
+		buf = smalloc(Maxstring);
+		if(waserror()){
+			free(buf);
+			nexterror();
+		}
 		x = f->p[PROTO(ch->qid)];
 		c = x->conv[CONV(ch->qid)];
 		if(x->local == nil) {
-			snprint(buf, Statelen, "%I!%d\n", c->laddr, c->lport);
+			snprint(buf, Maxstring, "%I!%d\n", c->laddr, c->lport);
 		} else {
-			(*x->local)(c, buf, Statelen-2);
+			(*x->local)(c, buf, Maxstring);
 		}
-		rv = readstr(offset, p, n, buf);
-		free(buf);
-		return rv;
+		goto Readstr;
 	case Qstatus:
-		buf = smalloc(Statelen);
+		buf = smalloc(Maxstate);
+		if(waserror()){
+			free(buf);
+			nexterror();
+		}
 		x = f->p[PROTO(ch->qid)];
 		c = x->conv[CONV(ch->qid)];
-		(*x->state)(c, buf, Statelen-2);
-		rv = readstr(offset, p, n, buf);
-		free(buf);
-		return rv;
+		(*x->state)(c, buf, Maxstate);
+		goto Readstr;
 	case Qdata:
 		c = f->p[PROTO(ch->qid)]->conv[CONV(ch->qid)];
 		return qread(c->rq, a, n);
@@ -696,11 +710,13 @@ ipread(Chan *ch, void *a, long n, vlong off)
 		x = f->p[PROTO(ch->qid)];
 		if(x->stats == nil)
 			error("stats not implemented");
-		buf = smalloc(Statelen);
-		(*x->stats)(x, buf, Statelen);
-		rv = readstr(offset, p, n, buf);
-		free(buf);
-		return rv;
+		buf = smalloc(Maxstats);
+		if(waserror()){
+			free(buf);
+			nexterror();
+		}
+		(*x->stats)(x, buf, Maxstats);
+		goto Readstr;
 	}
 }
 
-- 
cgit v1.2.3