From: Igor Böhm Date: Wed, 10 Nov 2021 13:01:38 +0000 Subject: [PATCH] rc: fix leaking runq->cmdfile To reproduce run the following on a terminal: 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: Another `runq->cmdfile` leak is present here (captured on a cpu server): 277 ├listen [tcp * /rc/bin/service ] 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 ] 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: 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` 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()); } 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: 277 ├listen [tcp * /rc/bin/service ] 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 ] 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 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. --- diff a7ec6ee4e8103bb448f902e9b6d0dc69629abc57 f439fbb27f17b1fdc2b933bfb339a1ffa1887592 --- a/sys/src/cmd/rc/exec.c Mon Nov 8 02:05:51 2021 +++ b/sys/src/cmd/rc/exec.c Wed Nov 10 14:01:38 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(); } --- a/sys/src/cmd/rc/subr.c Mon Nov 8 02:05:51 2021 +++ b/sys/src/cmd/rc/subr.c Wed Nov 10 14:01:38 2021 @@ -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; }