From: Igor Böhm <igor@9lab.org>
Date: Tue, 09 Nov 2021 12:33:08 +0000
Subject: [PATCH] rc: fix leaking runq->cmdfile


To reproduce run the following on a terminal:
<snip>
cpu% leak -s `{pstree | grep termrc | sed 1q | awk '{print $1}'}
src(0x00209a82); // 12
src(0x0020b2a6); // 1
cpu% acid `{pstree | grep termrc | sed 1q | awk '{print $1}'}
/proc/358/text:amd64 plan 9 executable
/sys/lib/acid/port
/sys/lib/acid/amd64
acid: src(0x0020b2a6)
/sys/src/cmd/rc/plan9.c:169
 164		if(runq->argv->words == 0)
 165			poplist();
 166		else {
 167			free(runq->cmdfile);
 168			int f = open(runq->argv->words->word, 0);
>169			runq->cmdfile = strdup(runq->argv->words->word);
 170			runq->lexline = 1;
 171			runq->pc--;
 172			popword();
 173			if(f>=0) execcmds(openfd(f));
 174		}
acid:
</snap>

Another `runq->cmdfile` leak is present here (capture on a cpu server):
<snip>
277         ├listen [tcp * /rc/bin/service <nil>]
321         │├listen [/net/tcp/2 tcp!*!80]
322         │├listen [/net/tcp/3 tcp!*!17019]
324         ││└rc [/net/tcp/5 tcp!185.64.155.70!3516]
334         ││ ├rc -li
382         ││ │└pstree
336         ││ └rc
338         ││  └cat
323         │└listen [/net/tcp/4 tcp!*!17020]
278         ├listen [tcp * /rc/bin/service.auth <nil>]
320         │└listen [/net/tcp/1 tcp!*!567]
381         └closeproc
cpu% leak -s 338
<stdin>:2: (error) mainmem used but not set
cpu% leak -s 336
src(0x00209a82); // 2
src(0x002051d2); // 1
cpu% acid 336
/proc/336/text:amd64 plan 9 executable
/sys/lib/acid/port
/sys/lib/acid/amd64
acid: src(0x002051d2)
/sys/src/cmd/rc/exec.c:1056
 1051
 1052	void
 1053	Xsrcfile(void)
 1054	{
 1055		free(runq->cmdfile);
>1056		runq->cmdfile = strdup(runq->code[runq->pc++].s);
 1057	}
acid:
</snap>

These leaks happen because we do not free cmdfile on all execution paths
where `Xreturn()` is invoked. In `/sys/src/cmd/rc/exec.c:/^Xreturn`

<snip>
void
Xreturn(void)
{
	struct thread *p = runq;
	turfredir();
	while(p->argv) poplist();
	codefree(p->code);
	runq = p->ret;
	free(p);
	if(runq==0)
		Exit(getstatus());
}
</snip>

Note how the function `Xreturn()` frees a heap allocated instance with its
members *except* the `cmdfile` member.

On some code paths where `Xreturn()` is called there is an attempt to free
`cmdfile`, however, there are some code paths where `Xreturn()` is called
where `cmdfile` is not freed leading to a leak.

The attached patch calls `free(p->cmdfile)` in `Xreturn()` to avoid leaking
memory and handling the free in one place.

After applying the patch this particular leak is removed. There are still
other leaks in rc:

<snip>
277         ├listen [tcp * /rc/bin/service <nil>]
321         │├listen [/net/tcp/2 tcp!*!80]
322         │├listen [/net/tcp/3 tcp!*!17019]
324         ││└rc [/net/tcp/5 tcp!185.64.155.70!3516]
334         ││ ├rc -li
382         ││ │└pstree
336         ││ └rc
338         ││  └cat
323         │└listen [/net/tcp/4 tcp!*!17020]
278         ├listen [tcp * /rc/bin/service.auth <nil>]
320         │└listen [/net/tcp/1 tcp!*!567]
381         └closeproc
cpu% leak -s 338
<stdin>:2: (error) mainmem used but not set
cpu% leak -s 336
src(0x00209a82); // 2
src(0x002051d2); // 1
cpu% acid 336
/proc/336/text:amd64 plan 9 executable
/sys/lib/acid/port
/sys/lib/acid/amd64
acid: src(0x00209a82)
/sys/src/cmd/rc/subr.c:9
 4	#include "fns.h"
 5
 6	void *
 7	emalloc(long n)
 8	{
>9		void *p = malloc(n);
 10		if(p==0)
 11			panic("Can't malloc %d bytes", n);
 12		return p;
 13	}
 14
</snap>

...fixing those will hopefully follow soon.
---
diff a7ec6ee4e8103bb448f902e9b6d0dc69629abc57 0e2fed4e4b042109f86ee6006ecdaa63077c669c
--- a/sys/src/cmd/rc/exec.c	Mon Nov  8 02:05:51 2021
+++ b/sys/src/cmd/rc/exec.c	Tue Nov  9 13:33:08 2021
@@ -465,6 +465,7 @@
 	turfredir();
 	while(p->argv) poplist();
 	codefree(p->code);
+	free(p->cmdfile);
 	runq = p->ret;
 	free(p);
 	if(runq==0)
@@ -937,8 +938,6 @@
 	Noerror();
 	if(yyparse()){
 		if(!p->iflag || p->eof && !Eintr()){
-			if(p->cmdfile)
-				free(p->cmdfile);
 			closeio(p->cmdfd);
 			Xreturn();
 		}