[PATCH] ash: preserve characters on heap in backquote parsing

[Denys Vlasenko]

Hi Ron,

As usual, since those years back when dash resumed (semi-)active
maintenance of ash codebase, I would like to be easily
pick up fixes from them.

In order to do that, it would be very useful to keep codebase
from diverging further than it has already diverged.

For example, bbox ash had a fix for nested override envvars.
When dash also fixed the same bug, in a different way,
I had to spend time ripping out my fix, then porting theirs.
PITA.

The location you are touching here is the same in dash.
Looks like it needs to be fixed there as well.
Can you submit this fix to dash? Then I'll port it from there,
and code will stay in sync.


On Sun, Nov 11, 2018 at 11:34 AM Ron Yorston <rmy at pobox.com> wrote:
>
> This bug:
>
>    https://bugs.busybox.net/show_bug.cgi?id=9246
>
> demonstrates a test case where BusyBox ash segfaults due to a stack
> overflow.
>
> Instead of using alloca() to preserve characters keep them on the
> shell's heap-based stack.  With this change the test case returns:
>
>     $ busybox sh test_case
>     test_case: line 3141: syntax error: unterminated quoted string
>
> That is, the test runs to completion.  If the heap is reduced to 8MB:
>
>     $ ulimit -S -d 8192
>     $ busybox sh test_case
>     sh: out of memory
>
> Heap exhaustion is detected and the shell continues to run.
>
> function                                             old     new   delta
> readtoken1                                          3265    3203     -62
> ------------------------------------------------------------------------------
> (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-62)             Total: -62 bytes
>
> Signed-off-by: Ron Yorston <rmy at pobox.com>
> ---
>  shell/ash.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/shell/ash.c b/shell/ash.c
> index 90eaf6faf..5ee36db93 100644
> --- a/shell/ash.c
> +++ b/shell/ash.c
> @@ -12599,18 +12599,8 @@ parsebackq: {
>
>         str = NULL;
>         savelen = out - (char *)stackblock();
> -       if (savelen > 0) {
> -               /*
> -                * FIXME: this can allocate very large block on stack and SEGV.
> -                * Example:
> -                * echo "..<100kbytes>..`true` $(true) `true` ..."
> -                * allocates 100kb for every command subst. With about
> -                * a hundred command substitutions stack overflows.
> -                * With larger prepended string, SEGV happens sooner.
> -                */
> -               str = alloca(savelen);
> -               memcpy(str, stackblock(), savelen);
> -       }
> +       if (savelen > 0)
> +               str = grabstackstr(out);
>
>         if (oldstyle) {
>                 /* We must read until the closing backquote, giving special
> --
> 2.19.1
>
> _______________________________________________
> busybox mailing list
> busybox at busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox