| Message ID | 20190718153028.18791-1-andrew.shadura@collabora.co.uk (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | dash: Fix stack overflow from infinite recursion in script | expand |
Andrej Shadura wrote in <20190718153028.18791-1-andrew.shadura@collabora\
.co.uk>:
|From: Chris Lamb <lamby@debian.org>
|
|Bug-Debian: https://bugs.debian.org/579815
|Signed-off-by: Chris Lamb <lamby@debian.org>
|Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
...
|+static int evalcount; /* number of nested evalfun calls */
...
|+ if (evalcount++ >= MAX_RECURSION)
|+ sh_error("Maximum function recursion depth (%d) \
...
|+#define MAX_RECURSION 1000 /* maximum recursion level */
A couple of years ago i discovered mksh was crashing due to not
having such a limit, and in the discussion which started with
Thorsten (the mksh developer) it became obvious that a then-new
bash seemed to have stopped using a limit; Thorsten's point was
that the real limit can only be artitificial or very costly (if
i recall correctly). By then i sent Mr. Xu a message in private
with i think a summary of all that, but never got any response.
I know that the Lua scripting language seems be in the process of
introducing such a limit with the upcoming release.
--steffen
|
|Der Kragenbaer, The moon bear,
|der holt sich munter he cheerfully and one by one
|einen nach dem anderen runter wa.ks himself off
|(By Robert Gernhardt)
> > +static int evalcount; /* number of nested evalfun calls */ > ... > > + if (evalcount++ >= MAX_RECURSION) > > + sh_error("Maximum function recursion depth (%d) \ > ... > > +#define MAX_RECURSION 1000 /* maximum recursion level */ > > A couple of years ago i discovered mksh was crashing due to not > having such a limit, and in the discussion which started with > Thorsten (the mksh developer) it became obvious that a then-new > bash seemed to have stopped using a limit; Thorsten's point was > that the real limit can only be artitificial or very costly (if > i recall correctly). By then i sent Mr. Xu a message in private > with i think a summary of all that, but never got any response. > I know that the Lua scripting language seems be in the process of > introducing such a limit with the upcoming release. A recursion limit is not without precedent. CPython sets a platform- dependent recursion limit for this same purpose: https://docs.python.org/3/library/sys.html#sys.setrecursionlimit But indeed, the limit is "artificial" or arbitrary. It should just be "enough" that it isn't hit often with typical use.
Hi all, > > > + if (evalcount++ >= MAX_RECURSION) > > > + sh_error("Maximum function recursion depth (%d) \ > > ... > > > +#define MAX_RECURSION 1000 /* maximum recursion level */ […] > A recursion limit is not without precedent. CPython sets a platform- > dependent recursion limit for this same purpose: CPython's dumb-yet-highly-effective limit was actually my source of inspiration for this patch. Regards,
Op 18-07-19 om 20:27 schreef Jason Bowen: > A recursion limit is not without precedent. CPython sets a platform- > dependent recursion limit for this same purpose: > > https://docs.python.org/3/library/sys.html#sys.setrecursionlimit > > But indeed, the limit is "artificial" or arbitrary. It should just be > "enough" that it isn't hit often with typical use. The arbitrariness of a hard-coded limit is indeed a problem. OK, crashing with a segfault is ugly because it makes the user suspect the shell is buggy, not the script. On the other hand, with a 'reasonable' limit, a script might still segfault on very small systems, while that limit disallows full use of system resources on large systems. Regarding precedent: as far as I know, among POSIXy shells, a recursion limit is currently only implemented in zsh, with a user-settable FUNCNEST variable. If such a feature were to be implemented, I think a configurable limit is preferable to a hard-coded limit, and IMHO zsh's precedent should be followed (including the name of the variable). $ zsh -c 'f() { f; }; f' f: maximum nested function level reached; increase FUNCNEST? Of course, if you set FUNCNEST to a very large value, the stack will still overflow. $ zsh -c 'FUNCNEST=999999999999999999; f() { f; }; f' Segmentation fault: 11 Thanks, - M.
Op 18-07-19 om 22:15 schreef Martijn Dekker: > Regarding precedent: as far as I know, among POSIXy shells, a recursion > limit is currently only implemented in zsh, with a user-settable > FUNCNEST variable. I was mistaken: both AT&T ksh93 and NetBSD 8.1 sh have a hard-coded limit of 1000. - M.
Martijn Dekker <martijn@inlv.org> wrote: > > I was mistaken: both AT&T ksh93 and NetBSD 8.1 sh have a hard-coded > limit of 1000. Which still doesn't help you when you set the ulimit stack limit to some ridiculously small value. Such a limit in the presence of variable stack ulimits is just wrong. Cheers,
diff --git a/src/eval.c b/src/eval.c index 1aad31a..ee36c39 100644 --- a/src/eval.c +++ b/src/eval.c @@ -69,6 +69,7 @@ int evalskip; /* set if we are skipping commands */ STATIC int skipcount; /* number of levels to skip */ MKINIT int loopnest; /* current loop nesting level */ static int funcline; /* starting line number of current function, or 0 if not in a function */ +static int evalcount; /* number of nested evalfun calls */ char *commandname; @@ -903,7 +904,12 @@ raise: break; case CMDFUNCTION: - if (evalfun(cmdentry.u.func, argc, argv, flags)) + if (evalcount++ >= MAX_RECURSION) + sh_error("Maximum function recursion depth (%d) reached", + MAX_RECURSION); + int i = evalfun(cmdentry.u.func, argc, argv, flags); + evalcount--; + if (i) goto raise; break; } diff --git a/src/eval.h b/src/eval.h index 63e7d86..38dffbd 100644 --- a/src/eval.h +++ b/src/eval.h @@ -51,6 +51,8 @@ struct backcmd { /* result of evalbackcmd */ #define EV_EXIT 01 /* exit after evaluating tree */ #define EV_TESTED 02 /* exit status is checked; ignore -e flag */ +#define MAX_RECURSION 1000 /* maximum recursion level */ + int evalstring(char *, int); union node; /* BLETCH for ansi C */ int evaltree(union node *, int);