From efa6937460beac50fe7261e817ca80f9bbb64d9d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Igor=20B=C3=B6hm?= <igor@9lab.org>
Date: Fri, 5 Nov 2021 23:51:55 +0000
Subject: [PATCH] acme: fix leaking memory allocated by getenv("font")
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If the font chosen for acme is retrieved via `getenv("font")` its
memory is leaked:

<snip>
	if(fontnames[0] == nil)
		fontnames[0] = getenv("font");
		^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> getenv(…) mallocs memory

	if(fontnames[0] == nil)
		fontnames[0] = "/lib/font/bit/vga/unicode.font";
	if(access(fontnames[0], 0) < 0){
		fprint(2, "acme: can't access %s: %r\n", fontnames[0]);
		exits("font open");
	}
	if(fontnames[1] == nil)
		fontnames[1] = fontnames[0];
	fontnames[0] = estrdup(fontnames[0]);
	^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> if the `getenv("font")` path was taken above, this assignment
> will leak its memory.
</snap>

The following leak/acid session demonstrates the issue:

<snip>
cpu% leak -s 212252
src(0x002000cb); // 1
cpu% acid 212252
/proc/212252/text:amd64 plan 9 executable
/sys/lib/acid/port
/sys/lib/acid/amd64
acid: src(0x002000cb)
/sys/src/cmd/acme/acme.c:107
 102			fprint(2, "usage: acme [-aib] [-c ncol] [-f font] [-F fixedfont] [-l loadfile | file...]\n");
 103			exits("usage");
 104		}ARGEND
 105
 106		if(fontnames[0] == nil)
>107			fontnames[0] = getenv("font");
 108		if(fontnames[0] == nil)
 109			fontnames[0] = "/lib/font/bit/vga/unicode.font";
 110		if(access(fontnames[0], 0) < 0){
 111			fprint(2, "acme: can't access %s: %r\n", fontnames[0]);
 112			exits("font open");
acid:
</snap>

The fix tries to first check if a font has been set via
command line options in which case the font string is
malloced via estrdup(…).

If no font has been selected on the command line getenv("font")
is used. If no getenv("font") var is found we malloc a default
font via estrdup(…).

<snip>
	if(fontnames[0] != nil)
		fontnames[0] = estrdup(fontnames[0]);
	else
		if((fontnames[0] = getenv("font")) == nil)
			fontnames[0] = estrdup("/lib/font/bit/vga/unicode.font");
	if(access(fontnames[0], 0) < 0){
		fprint(2, "acme: can't access %s: %r\n", fontnames[0]);
		exits("font open");
	}
	if(fontnames[1] == nil)
		fontnames[1] = fontnames[0];
	fontnames[1] = estrdup(fontnames[1]);
</snap>

This resolves the memory leak reported by leak(1).
---
 sys/src/cmd/acme/acme.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/sys/src/cmd/acme/acme.c b/sys/src/cmd/acme/acme.c
index 38d676f17..a2429b05b 100644
--- a/sys/src/cmd/acme/acme.c
+++ b/sys/src/cmd/acme/acme.c
@@ -103,17 +103,17 @@ threadmain(int argc, char *argv[])
 		exits("usage");
 	}ARGEND
 
-	if(fontnames[0] == nil)
-		fontnames[0] = getenv("font");
-	if(fontnames[0] == nil)
-		fontnames[0] = "/lib/font/bit/vga/unicode.font";
+	if(fontnames[0] != nil)
+		fontnames[0] = estrdup(fontnames[0]);
+	else
+		if((fontnames[0] = getenv("font")) == nil)
+			fontnames[0] = estrdup("/lib/font/bit/vga/unicode.font");		
 	if(access(fontnames[0], 0) < 0){
 		fprint(2, "acme: can't access %s: %r\n", fontnames[0]);
 		exits("font open");
 	}
 	if(fontnames[1] == nil)
 		fontnames[1] = fontnames[0];
-	fontnames[0] = estrdup(fontnames[0]);
 	fontnames[1] = estrdup(fontnames[1]);
 
 	quotefmtinstall();