From da2e46dff6576c29fa1d379c943bb7943aa6e7ce Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 21 Feb 2020 15:25:37 +0100 Subject: ash: memalloc: Avoid looping in growstackto Upstream commit: Date: Thu, 31 May 2018 01:51:48 +0800 memalloc: Avoid looping in growstackto Currently growstackto will repeatedly call growstackblock until the requisite size is obtained. This is wasteful. This patch changes growstackblock to take a minimum size instead. Signed-off-by: Herbert Xu Signed-off-by: Denys Vlasenko --- shell/ash.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/shell/ash.c b/shell/ash.c index fd2fc9f23..8d1822847 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -1678,15 +1678,16 @@ popstackmark(struct stackmark *mark) * part of the block that has been used. */ static void -growstackblock(void) +growstackblock(size_t min) { size_t newlen; newlen = g_stacknleft * 2; if (newlen < g_stacknleft) ash_msg_and_raise_error(bb_msg_memory_exhausted); - if (newlen < 128) - newlen += 128; + min = SHELL_ALIGN(min | 128); + if (newlen < min) + newlen += min; if (g_stacknxt == g_stackp->space && g_stackp != &stackbase) { struct stack_block *sp; @@ -1736,16 +1737,15 @@ static void * growstackstr(void) { size_t len = stackblocksize(); - growstackblock(); + growstackblock(0); return (char *)stackblock() + len; } static char * growstackto(size_t len) { - while (stackblocksize() < len) - growstackblock(); - + if (stackblocksize() < len) + growstackblock(len); return stackblock(); } -- cgit v1.2.3-55-g6feb From 45dd87aac05ddf8bbfb110fde85ef875f3adfb65 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 21 Feb 2020 16:30:44 +0100 Subject: ash: expand: Ensure result is escaped in cvtnum Upstream commit: Date: Fri, 1 Jun 2018 18:25:29 +0800 expand: Ensure result is escaped in cvtnum The minus sign generated from arithmetic expansion is currently unquoted which causes anomalies when the result is used in where the quoting matters. This patch fixes it by explicitly calling memtodest on the result in cvtnum. Signed-off-by: Herbert Xu Signed-off-by: Denys Vlasenko --- shell/ash.c | 49 +++++++++++------------ shell/ash_test/ash-quoting/negative_arith.right | 2 + shell/ash_test/ash-quoting/negative_arith.tests | 8 ++++ shell/hush_test/hush-quoting/negative_arith.right | 2 + shell/hush_test/hush-quoting/negative_arith.tests | 8 ++++ 5 files changed, 44 insertions(+), 25 deletions(-) create mode 100644 shell/ash_test/ash-quoting/negative_arith.right create mode 100755 shell/ash_test/ash-quoting/negative_arith.tests create mode 100644 shell/hush_test/hush-quoting/negative_arith.right create mode 100755 shell/hush_test/hush-quoting/negative_arith.tests diff --git a/shell/ash.c b/shell/ash.c index 8d1822847..005d87ecf 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -6070,26 +6070,6 @@ static struct ifsregion *ifslastp; /* holds expanded arg list */ static struct arglist exparg; -/* - * Our own itoa(). - * cvtnum() is used even if math support is off (to prepare $? values and such). - */ -static int -cvtnum(arith_t num) -{ - int len; - - /* 32-bit and wider ints require buffer size of bytes*3 (or less) */ - len = sizeof(arith_t) * 3; - /* If narrower: worst case, 1-byte ints: need 5 bytes: "-127" */ - if (sizeof(arith_t) < 4) len += 2; - - expdest = makestrspace(len, expdest); - len = fmtstr(expdest, len, ARITH_FMT, num); - STADJUST(len, expdest); - return len; -} - /* * Break the argument string into pieces based upon IFS and add the * strings to the argument list. The regions of the string to be @@ -6347,16 +6327,18 @@ preglob(const char *pattern, int flag) /* * Put a string on the stack. */ -static void +static size_t memtodest(const char *p, size_t len, int flags) { int syntax = flags & EXP_QUOTED ? DQSYNTAX : BASESYNTAX; char *q; + char *s; if (!len) - return; + return 0; q = makestrspace(len * 2, expdest); + s = q; do { unsigned char c = *p++; @@ -6375,6 +6357,7 @@ memtodest(const char *p, size_t len, int flags) } while (--len); expdest = q; + return q - s; } static size_t @@ -6385,6 +6368,22 @@ strtodest(const char *p, int flags) return len; } +/* + * Our own itoa(). + * cvtnum() is used even if math support is off (to prepare $? values and such). + */ +static int +cvtnum(arith_t num, int flags) +{ + /* 32-bit and wider ints require buffer size of bytes*3 (or less) */ + /* If narrower: worst case, 1-byte ints: need 5 bytes: "-127" */ + int len = (sizeof(arith_t) >= 4) ? sizeof(arith_t) * 3 : sizeof(arith_t) * 3 + 2; + char buf[len]; + + len = fmtstr(buf, len, ARITH_FMT, num); + return memtodest(buf, len, flags); +} + /* * Record the fact that we have to scan this region of the * string for IFS characters. @@ -6683,7 +6682,7 @@ expari(int flag) if (flag & QUOTES_ESC) rmescapes(p + 1, 0, NULL); - len = cvtnum(ash_arith(p + 1)); + len = cvtnum(ash_arith(p + 1), flag); if (!(flag & EXP_QUOTED)) recordregion(begoff, begoff + len, 0); @@ -7328,7 +7327,7 @@ varvalue(char *name, int varflags, int flags, int quoted) if (num == 0) return -1; numvar: - len = cvtnum(num); + len = cvtnum(num, flags); goto check_1char_name; case '-': expdest = makestrspace(NOPTS, expdest); @@ -7494,7 +7493,7 @@ evalvar(char *p, int flag) varunset(p, var, 0, 0); if (subtype == VSLENGTH) { - cvtnum(varlen > 0 ? varlen : 0); + cvtnum(varlen > 0 ? varlen : 0, flag); goto record; } diff --git a/shell/ash_test/ash-quoting/negative_arith.right b/shell/ash_test/ash-quoting/negative_arith.right new file mode 100644 index 000000000..e7e51eee7 --- /dev/null +++ b/shell/ash_test/ash-quoting/negative_arith.right @@ -0,0 +1,2 @@ +tempfile0.tmp tempfile9.tmp +tempfile0.tmp tempfile1.tmp tempfile9.tmp diff --git a/shell/ash_test/ash-quoting/negative_arith.tests b/shell/ash_test/ash-quoting/negative_arith.tests new file mode 100755 index 000000000..8e47ca111 --- /dev/null +++ b/shell/ash_test/ash-quoting/negative_arith.tests @@ -0,0 +1,8 @@ +>tempfile0.tmp +>tempfile1.tmp +>tempfile9.tmp +# The [...] is interpreted as: "any of the chars 0, -, and 9" +echo tempfile[0"$((-9))"].tmp +# The [...] is [0-9], interpreted as: "any digit" +echo tempfile[0$((-9))].tmp +rm tempfile?.tmp diff --git a/shell/hush_test/hush-quoting/negative_arith.right b/shell/hush_test/hush-quoting/negative_arith.right new file mode 100644 index 000000000..e7e51eee7 --- /dev/null +++ b/shell/hush_test/hush-quoting/negative_arith.right @@ -0,0 +1,2 @@ +tempfile0.tmp tempfile9.tmp +tempfile0.tmp tempfile1.tmp tempfile9.tmp diff --git a/shell/hush_test/hush-quoting/negative_arith.tests b/shell/hush_test/hush-quoting/negative_arith.tests new file mode 100755 index 000000000..8e47ca111 --- /dev/null +++ b/shell/hush_test/hush-quoting/negative_arith.tests @@ -0,0 +1,8 @@ +>tempfile0.tmp +>tempfile1.tmp +>tempfile9.tmp +# The [...] is interpreted as: "any of the chars 0, -, and 9" +echo tempfile[0"$((-9))"].tmp +# The [...] is [0-9], interpreted as: "any digit" +echo tempfile[0$((-9))].tmp +rm tempfile?.tmp -- cgit v1.2.3-55-g6feb From e4a0612efdedca0182d444942c34317e9df28a27 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 21 Feb 2020 17:21:34 +0100 Subject: hush: fix negative_arith.tests: glob-protect dash in "$((arith))" function old new delta expand_vars_to_list 1026 1082 +56 parse_dollar 810 811 +1 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 2/0 up/down: 57/0) Total: 57 bytes Signed-off-by: Denys Vlasenko --- shell/hush.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/shell/hush.c b/shell/hush.c index 357a354e2..0a92f5da7 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -3007,7 +3007,10 @@ static void x_mode_flush(void) static void o_addqchr(o_string *o, int ch) { int sz = 1; - char *found = strchr("*?[\\" MAYBE_BRACES, ch); + /* '-' is included because of this case: + * >filename0 >filename1 >filename9; v='-'; echo filename[0"$v"9] + */ + char *found = strchr("*?[-\\" MAYBE_BRACES, ch); if (found) sz++; o_grow_by(o, sz); @@ -3024,7 +3027,7 @@ static void o_addQchr(o_string *o, int ch) { int sz = 1; if ((o->o_expflags & EXP_FLAG_ESC_GLOB_CHARS) - && strchr("*?[\\" MAYBE_BRACES, ch) + && strchr("*?[-\\" MAYBE_BRACES, ch) ) { sz++; o->data[o->length] = '\\'; @@ -3041,7 +3044,7 @@ static void o_addqblock(o_string *o, const char *str, int len) while (len) { char ch; int sz; - int ordinary_cnt = strcspn(str, "*?[\\" MAYBE_BRACES); + int ordinary_cnt = strcspn(str, "*?[-\\" MAYBE_BRACES); if (ordinary_cnt > len) /* paranoia */ ordinary_cnt = len; o_addblock(o, str, ordinary_cnt); @@ -3052,7 +3055,7 @@ static void o_addqblock(o_string *o, const char *str, int len) ch = *str++; sz = 1; - if (ch) { /* it is necessarily one of "*?[\\" MAYBE_BRACES */ + if (ch) { /* it is necessarily one of "*?[-\\" MAYBE_BRACES */ sz++; o->data[o->length] = '\\'; o->length++; @@ -5049,7 +5052,7 @@ static int parse_dollar(o_string *as_string, ch = i_getch(input); nommu_addchr(as_string, ch); o_addchr(dest, SPECIAL_VAR_SYMBOL); - o_addchr(dest, /*quote_mask |*/ '+'); + o_addchr(dest, quote_mask | '+'); if (!BB_MMU) pos = dest->length; if (!add_till_closing_bracket(dest, input, ')' | DOUBLE_CLOSE_CHAR_FLAG)) @@ -6851,6 +6854,17 @@ static NOINLINE int expand_vars_to_list(o_string *output, int n, char *arg) res = expand_and_evaluate_arith(arg, NULL); debug_printf_subst("ARITH RES '"ARITH_FMT"'\n", res); sprintf(arith_buf, ARITH_FMT, res); + if (res < 0 + && first_ch == (char)('+'|0x80) + /* && (output->o_expflags & EXP_FLAG_ESC_GLOB_CHARS) */ + ) { + /* Quoted negative ariths, like filename[0"$((-9))"], + * should not be interpreted as glob ranges. + * Convert leading '-' to '\-': + */ + o_grow_by(output, 1); + output->data[output->length++] = '\\'; + } o_addstr(output, arith_buf); break; } -- cgit v1.2.3-55-g6feb From 9a1a659707a18c29166c3e2977523102866d7aed Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 22 Feb 2020 16:39:27 +0100 Subject: ash: parser: Fix old-style command substitution here-document crash Upstream commit: Date: Fri, 29 Mar 2019 13:49:59 +0800 parser: Fix old-style command substitution here-document crash ... This is caused by the recent change to save/restore here-docment list around command substitutions. In doing so we must finish existing here-documents prior to restoring the old here-document list. This is done for new-style command substitutions but not for old-style. This patch fixes it by doing it for both. Fixes: 51e2d88d6e51 ("parser: Save/restore here-documents in...") Signed-off-by: Herbert Xu Signed-off-by: Denys Vlasenko --- shell/ash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/ash.c b/shell/ash.c index 005d87ecf..83cac3fb0 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -12877,9 +12877,9 @@ parsebackq: { if (readtoken() != TRP) raise_error_unexpected_syntax(TRP); setinputstring(nullstr); - parseheredoc(); } + parseheredoc(); heredoclist = saveheredoclist; (*nlpp)->n = n; -- cgit v1.2.3-55-g6feb From c08993f40c0c6c7bdf453e77c3aa9dae8ec0dad9 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 22 Feb 2020 17:26:23 +0100 Subject: ash: parser: Do not push token back before parseheredoc Upstream commit: Date: Mon, 19 Nov 2018 18:43:58 +0800 parser: Do not push token back before parseheredoc When we read the first token in list() we use peektoken instead of readtoken as the following code needs to use the same token again. However, this is wrong when we're in a here-document as it will clobber the saved token without resetting the tokpushback flag. This patch fixes it by doing the tokpushback after parseheredoc and setting lasttoken again if parseheredoc was called. Reported-by: Ron Yorston Fixes: 7c245aa8ed33 ("[PARSER] Simplify EOF/newline handling in...") Fixes: ee5cbe9fd6bc ("[SHELL] Optimize dash -c "command" to avoid a fork") Signed-off-by: Herbert Xu Tested-by: Simon Ser Signed-off-by: Denys Vlasenko --- shell/ash.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/shell/ash.c b/shell/ash.c index 83cac3fb0..5fb67c0fa 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -11607,7 +11607,7 @@ list(int nlflag) n1 = NULL; for (;;) { - switch (peektoken()) { + switch (readtoken()) { case TNL: if (!(nlflag & 1)) break; @@ -11618,9 +11618,12 @@ list(int nlflag) if (!n1 && (nlflag & 1)) n1 = NODE_EOF; parseheredoc(); + tokpushback++; + lasttoken = TEOF; return n1; } + tokpushback++; checkkwd = CHKNL | CHKKWD | CHKALIAS; if (nlflag == 2 && ((1 << peektoken()) & tokendlist)) return n1; -- cgit v1.2.3-55-g6feb From c2058ec98cf3f6722be4436cae07a386e3c7b48d Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 22 Feb 2020 20:25:03 +0100 Subject: ash: Expand here-documents in the current shell environment Upstream commit: Date: Sun, 11 Nov 2007 15:27:00 +0800 Expand here-documents in the current shell environment Previously we always expanded here-documents in a subshell. This is contrary to the POSIX specification and how other shells behave. What's more this slows down many expansions due to the extra fork (however, it must be said that it is possible for it speed up certain expansions by running it simultaneously with the command on two CPUs). This patch move the expansion into the current shell environment. Test case: unset a cat <<- EOF > /dev/null ${a=NOT} EOF echo ${a}BAD Old result: BAD New result: NOTBAD Signed-off-by: Denys Vlasenko --- shell/ash.c | 29 ++++++++++++---------- .../ash-heredoc/heredoc_side_effects.right | 1 + .../ash-heredoc/heredoc_side_effects.tests | 5 ++++ 3 files changed, 22 insertions(+), 13 deletions(-) create mode 100644 shell/ash_test/ash-heredoc/heredoc_side_effects.right create mode 100755 shell/ash_test/ash-heredoc/heredoc_side_effects.tests diff --git a/shell/ash.c b/shell/ash.c index 5fb67c0fa..01346108a 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -5444,22 +5444,29 @@ stoppedjobs(void) * the pipe without forking. */ /* openhere needs this forward reference */ -static void expandhere(union node *arg, int fd); +static void expandhere(union node *arg); static int openhere(union node *redir) { + char *p; int pip[2]; size_t len = 0; if (pipe(pip) < 0) ash_msg_and_raise_perror("can't create pipe"); - if (redir->type == NHERE) { - len = strlen(redir->nhere.doc->narg.text); - if (len <= PIPE_BUF) { - full_write(pip[1], redir->nhere.doc->narg.text, len); - goto out; - } + + p = redir->nhere.doc->narg.text; + if (redir->type == NXHERE) { + expandhere(redir->nhere.doc); + p = stackblock(); } + + len = strlen(p); + if (len <= PIPE_BUF) { + xwrite(pip[1], p, len); + goto out; + } + if (forkshell((struct job *)NULL, (union node *)NULL, FORK_NOJOB) == 0) { /* child */ close(pip[0]); @@ -5468,10 +5475,7 @@ openhere(union node *redir) ignoresig(SIGHUP); //signal(SIGHUP, SIG_IGN); ignoresig(SIGTSTP); //signal(SIGTSTP, SIG_IGN); signal(SIGPIPE, SIG_DFL); - if (redir->type == NHERE) - full_write(pip[1], redir->nhere.doc->narg.text, len); - else /* NXHERE */ - expandhere(redir->nhere.doc, pip[1]); + xwrite(pip[1], p, len); _exit(EXIT_SUCCESS); } out: @@ -8016,10 +8020,9 @@ expandarg(union node *arg, struct arglist *arglist, int flag) * Expand shell variables and backquotes inside a here document. */ static void -expandhere(union node *arg, int fd) +expandhere(union node *arg) { expandarg(arg, (struct arglist *)NULL, EXP_QUOTED); - full_write(fd, stackblock(), expdest - (char *)stackblock()); } /* diff --git a/shell/ash_test/ash-heredoc/heredoc_side_effects.right b/shell/ash_test/ash-heredoc/heredoc_side_effects.right new file mode 100644 index 000000000..53b2c03c1 --- /dev/null +++ b/shell/ash_test/ash-heredoc/heredoc_side_effects.right @@ -0,0 +1 @@ +NO BUG diff --git a/shell/ash_test/ash-heredoc/heredoc_side_effects.tests b/shell/ash_test/ash-heredoc/heredoc_side_effects.tests new file mode 100755 index 000000000..3fb622ae4 --- /dev/null +++ b/shell/ash_test/ash-heredoc/heredoc_side_effects.tests @@ -0,0 +1,5 @@ +unset a +cat </dev/null +${a=NO} +EOF +echo $a BUG -- cgit v1.2.3-55-g6feb