Discussion:
[Bug 7748] New: ash: fix a memory leak
Yuki Machida
2018-11-21 01:39:59 UTC
Permalink
Hi all,

Please merge a patch to upstream of busybox.

Following patch is applied to busybobx 03ad7ae08.
(Modified with reference to http://lists.busybox.net/pipermail/busybox-cvs/2014-December/034873.html)
I confirmed that fix a memory leak.

diff --git shell/ash.c shell/ash.c
index 04e4006c8..6bd849568 100644
--- shell/ash.c
+++ shell/ash.c
@@ -9049,6 +9049,7 @@ evaltree(union node *n, int flags)
int checkexit = 0;
int (*evalfn)(union node *, int);
int status = 0;
+ struct stackmark smark;

if (n == NULL) {
TRACE(("evaltree(NULL) called\n"));
@@ -9069,6 +9070,7 @@ evaltree(union node *n, int flags)
status = !evaltree(n->nnot.com, EV_TESTED);
goto setstatus;
case NREDIR:
+ setstackmark(&smark);
errlinno = lineno = n->nredir.linno;
if (funcline)
lineno -= funcline - 1;
@@ -9080,6 +9082,7 @@ evaltree(union node *n, int flags)
}
if (n->nredir.redirect)
popredir(/*drop:*/ 0);
+ popstackmark(&smark);
goto setstatus;
case NCMD:
evalfn = evalcommand;

Best regards,
Yuki Machida
Ron Yorston
2018-11-22 12:34:06 UTC
Permalink
There's another case where a leak is possible: when the redirection
is associated with a subshell inside the long-running loop:

while true; do ( true; ) </dev/null; done

This case appears to be fixed by the extended patch below.

The same problems are also present in dash and can be fixed by a similar
patch. Bash is unaffected.

I'm still not sure if these patches are safe.

Ron

---
shell/ash.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/shell/ash.c b/shell/ash.c
index 04e4006c8..329ddae67 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -9049,6 +9049,7 @@ evaltree(union node *n, int flags)
int checkexit = 0;
int (*evalfn)(union node *, int);
int status = 0;
+ struct stackmark smark;

if (n == NULL) {
TRACE(("evaltree(NULL) called\n"));
@@ -9069,6 +9070,7 @@ evaltree(union node *n, int flags)
status = !evaltree(n->nnot.com, EV_TESTED);
goto setstatus;
case NREDIR:
+ setstackmark(&smark);
errlinno = lineno = n->nredir.linno;
if (funcline)
lineno -= funcline - 1;
@@ -9080,6 +9082,7 @@ evaltree(union node *n, int flags)
}
if (n->nredir.redirect)
popredir(/*drop:*/ 0);
+ popstackmark(&smark);
goto setstatus;
case NCMD:
evalfn = evalcommand;
@@ -9298,7 +9301,9 @@ evalsubshell(union node *n, int flags)
struct job *jp;
int backgnd = (n->type == NBACKGND); /* FORK_BG(1) if yes, else FORK_FG(0) */
int status;
+ struct stackmark smark;

+ setstackmark(&smark);
errlinno = lineno = n->nredir.linno;
if (funcline)
lineno -= funcline - 1;
@@ -9326,6 +9331,7 @@ evalsubshell(union node *n, int flags)
if (backgnd == FORK_FG)
status = waitforjob(jp);
INT_ON;
+ popstackmark(&smark);
return status;
}
--
2.19.1
Yuki Machida
2018-11-28 05:15:14 UTC
Permalink
Hi Ron,

I confirmed that it disappeard a memory leak after apply this patch.

Best regards,
Yuki
Post by Ron Yorston
There's another case where a leak is possible: when the redirection
while true; do ( true; ) </dev/null; done
This case appears to be fixed by the extended patch below.
The same problems are also present in dash and can be fixed by a similar
patch. Bash is unaffected.
I'm still not sure if these patches are safe.
Ron
---
shell/ash.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/shell/ash.c b/shell/ash.c
index 04e4006c8..329ddae67 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -9049,6 +9049,7 @@ evaltree(union node *n, int flags)
int checkexit = 0;
int (*evalfn)(union node *, int);
int status = 0;
+ struct stackmark smark;
if (n == NULL) {
TRACE(("evaltree(NULL) called\n"));
@@ -9069,6 +9070,7 @@ evaltree(union node *n, int flags)
status = !evaltree(n->nnot.com, EV_TESTED);
goto setstatus;
+ setstackmark(&smark);
errlinno = lineno = n->nredir.linno;
if (funcline)
lineno -= funcline - 1;
@@ -9080,6 +9082,7 @@ evaltree(union node *n, int flags)
}
if (n->nredir.redirect)
popredir(/*drop:*/ 0);
+ popstackmark(&smark);
goto setstatus;
evalfn = evalcommand;
@@ -9298,7 +9301,9 @@ evalsubshell(union node *n, int flags)
struct job *jp;
int backgnd = (n->type == NBACKGND); /* FORK_BG(1) if yes, else FORK_FG(0) */
int status;
+ struct stackmark smark;
+ setstackmark(&smark);
errlinno = lineno = n->nredir.linno;
if (funcline)
lineno -= funcline - 1;
@@ -9326,6 +9331,7 @@ evalsubshell(union node *n, int flags)
if (backgnd == FORK_FG)
status = waitforjob(jp);
INT_ON;
+ popstackmark(&smark);
return status;
}
--
Yuki Machida
Loading...