Re: [PATCH] dash: Fix stack overflow from infinite recursion in script
Martijn Dekker wrote: > > I was mistaken: both AT 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, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] dash: Fix stack overflow from infinite recursion in script
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=99; f() { f; }; f'
Segmentation fault: 11
Thanks,
- M.
--
modernish -- harness the shell
https://github.com/modernish/modernish
Re: [PATCH] dash: Fix stack overflow from infinite recursion in script
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 ksh93 and NetBSD 8.1 sh have a hard-coded limit of 1000. - M. -- modernish -- harness the shell https://github.com/modernish/modernish
Re: [PATCH] dash: Fix stack overflow from infinite recursion in script
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,
--
,''`.
: :' : Chris Lamb
`. `'` la...@debian.org chris-lamb.co.uk
`-
Re: [PATCH] dash: Fix stack overflow from infinite recursion in script
> > +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.
Re: [PATCH] dash: Fix stack overflow from infinite recursion in script
Andrej Shadura wrote in <20190718153028.18791-1-andrew.shadura@collabora\
.co.uk>:
|From: Chris Lamb
|
|Bug-Debian: https://bugs.debian.org/579815
|Signed-off-by: Chris Lamb
|Signed-off-by: Andrej Shadura
...
|+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)
[PATCH] dash: Fix stack overflow from infinite recursion in script
From: Chris Lamb
Bug-Debian: https://bugs.debian.org/579815
Signed-off-by: Chris Lamb
Signed-off-by: Andrej Shadura
---
src/eval.c | 8 +++-
src/eval.h | 2 ++
2 files changed, 9 insertions(+), 1 deletion(-)
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);
--
2.20.1
