The following constructs result in ever-increasing memory usage:
while true; do { true; } </dev/null; done
while true; do ( true; ) </dev/null; done
For comparison, bash displays static memory usage in both cases.
This has been fixed in dash by commit 2bc6caa. The maintainer
writes:
I have simplified evaltree so that it simply sets the stack mark
unconditionally. This allows us to remove the stack marks in the
functions called by evaltree.
Closes BusyBox bug 7748.
function old new delta
evaltree 606 632 +26
evalcommand 1724 1696 -28
evalcase 382 351 -31
evalfor 230 196 -34
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/3 up/down: 26/-93) Total: -67 bytes
Signed-off-by: Ron Yorston <rmy@pobox.com>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
Embedded scripts require a shell to be present in the BusyBox
binary. Allow either ash or hush to be used for this purpose.
If both are enabled ash takes precedence.
The size of the binary is unchanged in the default configuration:
both ash and hush are present but support for embedded scripts
isn't compiled into hush.
Signed-off-by: Ron Yorston <rmy@pobox.com>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
The parser reads from an already freed memory location, thereby causing
unpredictable results, in the following situation:
- ENABLE_ASH_EXPAND_PRMT is enabled
- heredoc is being parsed
- command substitution is used within heredoc
Examples where this bug crops up are (PS2 is set to "> "):
$ cat <<EOF
> `echo abc`
> EOF
-sh: O: not found
$ cat <<EOF
> $(echo abc)
> EOF
-sh: {garbage}: not found
The presumable reason is that setprompt_if() causes a nested expansion when
ENABLE_ASH_EXPAND_PRMT is enabled, therefore leaving "wordtext" in an unusable
state. However, when parseheredoc() is called, "tokpushback" is non-zero, which
causes the next call to xxreadtoken() to return TWORD, causing the caller to
use the invalid "wordtoken" instead of reading the next valid token.
The call chain is:
list()
-> peektoken() [sets tokpushback to 1]
-> parseheredoc()
-> setprompt_if()
-> pushstackmark()
-> expandstr()
-> readtoken1()
[sets lasttoken to TWORD, wordtoken points to expanded prompt]
-> popstackmark() [invalidates wordtoken, leaves lasttoken as is]
-> readtoken1()
-> ...parsebackq
-> list()
-> andor()
-> pipeline()
-> readtoken()
-> xxreadtoken()
[tokpushback non-zero, reuse lasttoken and wordtext]
Note that in almost all other contexts, each call to setprompt_if() is preceded
by setting "tokpushback" to zero. One exception is "oldstyle" backquote parsing
in readtoken1(), but there "tokpushback" is reset afterwards. The other
exception is nlprompt(), but this function is only used within readtoken1()
(but in contexts where no nested calls to xxreadtoken() occur) and xxreadtoken()
(where "tokpushback" is guaranteed to be zero).
function old new delta
parseheredoc 124 131 +7
Signed-off-by: Christoph Schulz <develop@kristov.de>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
BusyBox has support for embedded shell scripts. Two types can be
distinguished: custom scripts and scripts implementing applets.
Custom scripts should be placed in the 'embed' directory at build
time. They are given a default applet configuration and appear
as applets to the user but no further configuration is possible.
Applet scripts are integrated with the BusyBox build system and
are intended to be used to ship standard applets that just happen
to be implemented as scripts. They can be configured at build time
and appear just like native applets.
Such scripts should be placed in the 'applets_sh' directory. A stub
C program should be written to provide the usual applet configuration
details and placed in a suitable subsystem directory. It may be
helpful to have a configuration option to enable any dependencies the
script requires: see the 'nologin' applet for an example.
function old new delta
scripted_main - 41 +41
applet_names 2773 2781 +8
applet_main 1600 1604 +4
i2cdetect_main 672 674 +2
applet_suid 100 101 +1
applet_install_loc 200 201 +1
applet_flags 100 101 +1
packed_usage 33180 33179 -1
tryexec 159 152 -7
evalcommand 1661 1653 -8
script_names 9 - -9
packed_scripts 123 114 -9
complete_cmd_dir_file 826 811 -15
shellexec 271 254 -17
find_command 1007 990 -17
busybox_main 642 624 -18
run_applet_and_exit 100 78 -22
find_script_by_name 51 - -51
------------------------------------------------------------------------------
(add/remove: 1/2 grow/shrink: 6/9 up/down: 58/-174) Total: -116 bytes
text data bss dec hex filename
950034 477 7296 957807 e9d6f busybox_old
949918 477 7296 957691 e9cfb busybox_unstripped
Signed-off-by: Ron Yorston <rmy@pobox.com>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
When a variable is unset by calling setvar(name, NULL, 0) the code
to initialise the new, empty variable fails to initialise the last
character of the string.
Attempts to read the contents of the unset variable will result
in the uninitialised character at the end of the string being
accessed.
For example, running BusyBox under Valgrind and unsetting PATH:
$ valgrind ./busybox_unstripped sh
==21249== Memcheck, a memory error detector
==21249== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==21249== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==21249== Command: ./busybox_unstripped sh
==21249==
/data2/git/build_fix_8721 $ unset PATH
/data2/git/build_fix_8721 $ 0
==21249== Conditional jump or move depends on uninitialised value(s)
==21249== at 0x451371: path_advance (ash.c:2555)
==21249== by 0x456E22: find_command (ash.c:13407)
==21249== by 0x458425: evalcommand (ash.c:10139)
==21249== by 0x454CBC: evaltree (ash.c:9131)
==21249== by 0x456C80: cmdloop (ash.c:13164)
Closes https://bugs.busybox.net/show_bug.cgi?id=8721
v2: On the dash mailing list Harald van Dijk was kind enough to point
out a flaw in my reasoning and provide an alternative patch. Sadly
his patch adds 2 bytes of bloat. Using xzalloc to zero the whole
string gives a bloat of -3 bytes.
function old new delta
setvar 172 169 -3
Signed-off-by: Ron Yorston <rmy@pobox.com>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
The HUSH_TICK configuration option enables command substitution,
not process substitution.
Signed-off-by: Ron Yorston <rmy@pobox.com>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
Ensure that login_sh is initialised in procargs even when running
an embedded script.
The argc argument to ash_main isn't unused when embedded scripts
are present.
Signed-off-by: Ron Yorston <rmy@pobox.com>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
To assist in the deployment of shell scripts it may be convenient
to embed them in the BusyBox binary.
'Embed scripts in the binary' takes any files in the directory
'embed', concatenates them with null separators, compresses them
and embeds them in the binary.
When scripts are embedded in the binary, scripts can be run as
'busybox SCRIPT [ARGS]' or by usual (sym)link mechanism.
embed/nologin is provided as an example.
function old new delta
packed_scripts - 123 +123
unpack_scripts - 87 +87
ash_main 1103 1171 +68
run_applet_and_exit 78 128 +50
get_script_content - 32 +32
script_names - 10 +10
expmeta 663 659 -4
------------------------------------------------------------------------------
(add/remove: 4/0 grow/shrink: 2/1 up/down: 370/-4) Total: 366 bytes
Signed-off-by: Ron Yorston <rmy@pobox.com>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
Upstream commit:
Date: Wed, 28 Mar 2018 18:37:51 +0800
expand: Do not quote backslashes in unquoted parameter expansion
Here is a better example:
a="/*/\nullx" b="/*/\null"; printf "%s\n" $a $b
dash currently prints
/*/\nullx
/*/\null
bash prints
/*/\nullx
/dev/null
You may argue the bash behaviour is inconsistent but it actually
makes sense. What happens is that quote removal only applies to
the original token as seen by the shell. It is never applied to
the result of parameter expansion.
Now you may ask why on earth does the second line say "/dev/null"
instead of "/dev/\null". Well that's because it is not the quote
removal step that removed the backslash, but the pathname expansion.
The fact that the /de\v does not become /dev even though it exists
is just the result of the optimisation to avoid unnecessarily
calling stat(2). I have checked POSIX and I don't see anything
that forbids this behaviour.
So going back to dash yes I think we should adopt the bash behaviour
for pathname expansion and keep the existing case semantics.
This patch does exactly that. Note that this patch does not work
unless you have already applied
https://patchwork.kernel.org/patch/10306507/
because otherwise the optimisation mentioned above does not get
detected correctly and we will end up doing quote removal twice.
This patch also updates expmeta to handle naked backslashes at
the end of the pattern which is now possible.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
function old new delta
expmeta 618 653 +35
memtodest 146 147 +1
Tested to work with both ASH_INTERNAL_GLOB on and off.
hush does not handle this correctly.
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
Upstream commit:
Date: Sat, 19 May 2018 02:39:37 +0800
exec: Return 126 on most errors in shellexec
Currently when shellexec fails on most errors the shell will exit
with exit status 2. This patch changes it to 126 in order to avoid
ambiguities with the exit status from a successful exec.
The errors that result in 127 has also been expanded to include
ENOTDIR, ENAMETOOLONG and ELOOP.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
function old new delta
shellexec 245 254 +9
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
Upstream commit:
Date: Tue, 27 Mar 2018 00:39:35 +0800
eval: Restore input files in evalcommand
When evalcommand invokes a command that modifies parsefile and
then bails out without popping the file, we need to ensure the
input file is restored so that the shell can continue to execute.
Reported-by: Martijn Dekker <martijn@inlv.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
function old new delta
unwindfiles - 20 +20
evalcommand 1635 1653 +18
getoptscmd 584 595 +11
popallfiles 20 10 -10
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 2/1 up/down: 49/-10) Total: 39 bytes
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
Upstream commit:
Date: Tue, 3 Apr 2018 00:40:25 +0800
parser: Fix parsing of ${}
dash -c 'echo ${}' should print "Bad subtitution" but instead
fails with "Syntax error: Missing '}'". This is caused by us
reading an extra character beyond the right brace. This patch
fixes it so that this construct only fails during expansion rather
than during parsing.
Fixes: 3df3edd13389 ("[PARSER] Report substition errors at...")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
function old new delta
readtoken1 2907 2916 +9
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
Upstream commit:
Date: Fri, 23 Mar 2018 18:58:47 +0800
expand: Fix ghost fields with unquoted $@/$*
You're right. The proper fix to this is to ensure that nulonly
is not set in varvalue for $*. It should only be set for $@ when
it's inside double quotes.
In fact there is another bug while we're playing with $@/$*.
When IFS is set to a non-whitespace character such as :, $*
outside quotes won't remove empty fields as it should.
This patch fixes both problems.
Reported-by: Martijn Dekker <martijn@inlv.org>
Suggested-by: Harald van Dijk <harald@gigawatt.nl>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
function old new delta
argstr 1111 1113 +2
evalvar 571 569 -2
varvalue 579 576 -3
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/2 up/down: 2/-5) Total: -3 bytes
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
Upstream commit:
Date: Sat, 19 May 2018 02:39:43 +0800
var: Set IFS to fixed value at start time
This patch forces the IFS variable to always be set to its default
value, regardless of the environment.
It also removes the long unused IFS_BROKEN code.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
Upstream commit:
Date: Wed, 4 Apr 2018 17:54:01 +0800
eval: Variable assignments on functions are no longer persistent
Dirk Fieldhouse <fieldhouse@gmx.net> wrote:
> In POSIX.1-2017 ("simultaneously IEEE Std 1003.1™-2017 and The Open
> Group Technical Standard Base Specifications, Issue 7")
> <http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09>,
> we read under '2.9.1 Simple Commands'
>
> "Variable assignments shall be performed as follows:
> ...
> - If the command name is a standard utility implemented as a function
> (see XBD Utility), the effect of variable assignments shall be as if the
> utility was not implemented as a function.
> ...
> - If the command name is a function that is not a standard utility
> implemented as a function, variable assignments shall affect the current
> execution environment during the execution of the function. It is
> unspecified:
>
> * Whether or not the variable assignments persist after the
> completion of the function
>
> * Whether or not the variables gain the export attribute during
> the execution of the function
>
> * Whether or not export attributes gained as a result of the
> variable assignments persist after the completion of the function (if
> variable assignments persist after the completion of the function)"
POSIX used to require the current dash behaviour. However, you're
right that this is no longer the case.
This patch will remove the persistence of the variable assignment.
I have considered the exporting the variables during the function
execution but have decided against it because:
1) It makes the code bigger.
2) dash has never done this in the past.
3) You cannot use this portably anyway.
Reported-by: Dirk Fieldhouse <fieldhouse@gmx.net>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
function old new delta
evalcommand 1606 1635 +29
evalcase 313 317 +4
evalfun 280 268 -12
pushlocalvars 48 - -48
------------------------------------------------------------------------------
(add/remove: 0/1 grow/shrink: 2/1 up/down: 33/-60) Total: -27 bytes
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
Was printing initial "+" prefix for the assignment, that printing
"+ cmd"
then printing the expanded " v=VAL" string.
Delay printing of "+" prefix for the assignment to after expansion.
function old new delta
run_pipe 1883 1902 +19
builtin_eval 127 133 +6
expand_vars_to_list 1103 1106 +3
dump_cmd_in_x_mode 144 142 -2
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 3/1 up/down: 28/-2) Total: 26 bytes
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
The n > 0 check to prevent access to the last byte of non-existing argv[-1]
wasn't enough. Switched to making sure there are initialized (zero) bytes there.
A predictable testcase is rather hard to construct, unfortunately,
contents of memory depends on allocator behavior and whatnot.
function old new delta
o_save_ptr_helper 119 137 +18
expand_on_ifs 345 339 -6
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/1 up/down: 18/-6) Total: 12 bytes
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
This necessitates switch from libc FILE api to a simple
homegrown replacement.
The change which fixes the bug here is the deleting of
restore_redirected_FILEs();
line. It was prematurely moving (restoring) script fd#3.
The fix is: we don't even _want_ to restore scrit fds,
we are perfectly fine with them being moved.
The only reason we tried to restore them is that FILE api
did not allow moving of FILE->fd.
function old new delta
refill_HFILE_and_getc - 93 +93
hfopen - 90 +90
hfclose - 66 +66
pseudo_exec_argv 591 597 +6
hush_main 1089 1095 +6
builtin_source 209 214 +5
save_fd_on_redirect 197 200 +3
setup_redirects 320 321 +1
fgetc_interactive 235 236 +1
i_peek_and_eat_bkslash_nl 99 97 -2
expand_vars_to_list 1103 1100 -3
restore_redirects 99 52 -47
fclose_and_forget 57 - -57
remember_FILE 63 - -63
------------------------------------------------------------------------------
(add/remove: 3/2 grow/shrink: 6/3 up/down: 271/-172) Total: 99 bytes
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
