shithub: riscv

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 (captured 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 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 of type `thread` 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 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> To help fixing those leaks emalloc(…) and erealloc(…) have been amended to use setmalloctag(…) and setrealloctag(…). The actual fixes for other reported leaks are *not* part of this merge and will follow.

--- a/sys/src/cmd/rc/exec.c

+++ b/sys/src/cmd/rc/exec.c

@@ -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();

--- a/sys/src/cmd/rc/subr.c

+++ b/sys/src/cmd/rc/subr.c

@@ -9,6 +9,7 @@

 	void *p = malloc(n);

 	if(p==0)

 		panic("Can't malloc %d bytes", n);

+	setmalloctag(p, getcallerpc(&n));

 	return p;

@@ -18,6 +19,7 @@

 	p = realloc(p, n);

 	if(p==0 && n!=0)

 		panic("Can't realloc %d bytes\n", n);

+	setrealloctag(p, getcallerpc(&p));

 	return p;