From 170f93ef1bc49a9e4c68f85f5245416767c1916f Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 29 Jul 2017 18:54:53 +0200 Subject: ash: "Undo all redirections" comment is wrong, delete it No code changes. Signed-off-by: Denys Vlasenko --- shell/ash.c | 48 ++++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 26 deletions(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index 0de81b325..02b21510e 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -5558,6 +5558,28 @@ redirect(union node *redir, int flags) preverrout_fd = copied_fd2; } +static int +redirectsafe(union node *redir, int flags) +{ + int err; + volatile int saveint; + struct jmploc *volatile savehandler = exception_handler; + struct jmploc jmploc; + + SAVE_INT(saveint); + /* "echo 9>/dev/null; echo >&9; echo result: $?" - result should be 1, not 2! */ + err = setjmp(jmploc.loc); // huh?? was = setjmp(jmploc.loc) * 2; + if (!err) { + exception_handler = &jmploc; + redirect(redir, flags); + } + exception_handler = savehandler; + if (err && exception_type != EXERROR) + longjmp(exception_handler->loc, 1); + RESTORE_INT(saveint); + return err; +} + /* * Undo the effects of the last redirection. */ @@ -5593,32 +5615,6 @@ popredir(int drop, int restore) INT_ON; } -/* - * Undo all redirections. Called on error or interrupt. - */ - -static int -redirectsafe(union node *redir, int flags) -{ - int err; - volatile int saveint; - struct jmploc *volatile savehandler = exception_handler; - struct jmploc jmploc; - - SAVE_INT(saveint); - /* "echo 9>/dev/null; echo >&9; echo result: $?" - result should be 1, not 2! */ - err = setjmp(jmploc.loc); // huh?? was = setjmp(jmploc.loc) * 2; - if (!err) { - exception_handler = &jmploc; - redirect(redir, flags); - } - exception_handler = savehandler; - if (err && exception_type != EXERROR) - longjmp(exception_handler->loc, 1); - RESTORE_INT(saveint); - return err; -} - /* ============ Routines to expand arguments to commands * -- cgit v1.2.3-55-g6feb From a732898fdd9748b966da228ee8bbbc148c3c10c9 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 29 Jul 2017 19:57:28 +0200 Subject: ash: [PARSER] Removed noexpand/length check on eofmark Upstream comment: Date: Sun, 11 Nov 2007 14:21:23 +0800 [PARSER] Removed noexpand/length check on eofmark On Tue, Oct 30, 2007 at 04:23:35AM +0000, Oleg Verych wrote: > > } 8<<"" > ====================== Actually this (the empty delim) only works with dash by accident. I've tried bash and pdksh and they both terminate on the first empty line which is what you would expect rather than EOF. The real Korn shell does something completely different. I've fixed this in dash to conform to bash/pdksh. > In [0] it's stated, that delimiter isn't evaluated (expanded), only > quoiting must be checked. That if() seems to be completely bogus. OK I agree. The reason it was there is because the parser would have already replaced the dollar sign by an internal representation. I've fixed it properly with this patch. Test case: cat <<- $a OK $a cat <<- "" OK echo OK Old result: dash: Syntax error: Illegal eof marker for << redirection OK echo OK New result: OK OK OK function old new delta parsefname 227 152 -75 readtoken1 2819 2651 -168 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-243) Total: -243 bytes Signed-off-by: Denys Vlasenko --- shell/ash.c | 111 +++++++++---------------- shell/ash_test/ash-heredoc/heredoc_empty.right | 3 + shell/ash_test/ash-heredoc/heredoc_empty.tests | 8 ++ 3 files changed, 52 insertions(+), 70 deletions(-) create mode 100644 shell/ash_test/ash-heredoc/heredoc_empty.right create mode 100755 shell/ash_test/ash-heredoc/heredoc_empty.tests (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index 02b21510e..2bfec83e6 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -9991,6 +9991,7 @@ static smallint checkkwd; #define CHKALIAS 0x1 #define CHKKWD 0x2 #define CHKNL 0x4 +#define CHKEOFMARK 0x8 /* * Push a string back onto the input at this current parsefile level. @@ -10302,31 +10303,6 @@ pgetc_without_PEOA(void) # define pgetc_without_PEOA() pgetc() #endif -/* - * Read a line from the script. - */ -static char * -pfgets(char *line, int len) -{ - char *p = line; - int nleft = len; - int c; - - while (--nleft > 0) { - c = pgetc_without_PEOA(); - if (c == PEOF) { - if (p == line) - return NULL; - break; - } - *p++ = c; - if (c == '\n') - break; - } - *p = '\0'; - return line; -} - /* * Undo a call to pgetc. Only two characters may be pushed back. * PEOF may be pushed back. @@ -10960,8 +10936,6 @@ raise_error_unexpected_syntax(int token) /* NOTREACHED */ } -#define EOFMARKLEN 79 - /* parsing is heavily cross-recursive, need these forward decls */ static union node *andor(void); static union node *pipeline(void); @@ -11141,43 +11115,22 @@ fixredir(union node *n, const char *text, int err) } } -/* - * Returns true if the text contains nothing to expand (no dollar signs - * or backquotes). - */ -static int -noexpand(const char *text) -{ - unsigned char c; - - while ((c = *text++) != '\0') { - if (c == CTLQUOTEMARK) - continue; - if (c == CTLESC) - text++; - else if (SIT(c, BASESYNTAX) == CCTL) - return 0; - } - return 1; -} - static void parsefname(void) { union node *n = redirnode; + if (n->type == NHERE) + checkkwd = CHKEOFMARK; if (readtoken() != TWORD) raise_error_unexpected_syntax(-1); if (n->type == NHERE) { struct heredoc *here = heredoc; struct heredoc *p; - int i; if (quoteflag == 0) n->type = NXHERE; TRACE(("Here document %d\n", n->type)); - if (!noexpand(wordtext) || (i = strlen(wordtext)) == 0 || i > EOFMARKLEN) - raise_error_syntax("illegal eof marker for << redirection"); rmescapes(wordtext, 0); here->eofmark = wordtext; here->next = NULL; @@ -11593,7 +11546,6 @@ readtoken1(int c, int syntax, char *eofmark, int striptabs) /* c parameter is an unsigned char or PEOF or PEOA */ char *out; size_t len; - char line[EOFMARKLEN + 1]; struct nodelist *bqlist; smallint quotef; smallint dblquote; @@ -11816,6 +11768,9 @@ readtoken1(int c, int syntax, char *eofmark, int striptabs) */ checkend: { if (eofmark) { + int markloc; + char *p; + #if ENABLE_ASH_ALIAS if (c == PEOA) c = pgetc_without_PEOA(); @@ -11825,27 +11780,42 @@ checkend: { c = pgetc_without_PEOA(); } } - if (c == *eofmark) { - if (pfgets(line, sizeof(line)) != NULL) { - char *p, *q; - int cc; - - p = line; - for (q = eofmark + 1;; p++, q++) { - cc = *p; - if (cc == '\n') - cc = 0; - if (!*q || cc != *q) - break; - } - if (cc == *q) { - c = PEOF; - nlnoprompt(); - } else { - pushstring(line, NULL); + + markloc = out - (char *)stackblock(); + for (p = eofmark; STPUTC(c, out), *p; p++) { + if (c != *p) + goto more_heredoc; + + c = pgetc_without_PEOA(); + } + + if (c == '\n' || c == PEOF) { + c = PEOF; + g_parsefile->linno++; + needprompt = doprompt; + } else { + int len_here; + + more_heredoc: + p = (char *)stackblock() + markloc + 1; + len_here = out - p; + + if (len_here) { + len_here -= (c >= PEOF); + c = p[-1]; + + if (len_here) { + char *str; + + str = alloca(len_here + 1); + *(char *)mempcpy(str, p, len_here) = '\0'; + + pushstring(str, NULL); } } } + + STADJUST((char *)stackblock() + markloc - out, out); } goto checkend_return; } @@ -11939,7 +11909,8 @@ parsesub: { int typeloc; c = pgetc_eatbnl(); - if (c > 255 /* PEOA or PEOF */ + if ((checkkwd & CHKEOFMARK) + || c > 255 /* PEOA or PEOF */ || (c != '(' && c != '{' && !is_name(c) && !is_special(c)) ) { #if BASH_DOLLAR_SQUOTE diff --git a/shell/ash_test/ash-heredoc/heredoc_empty.right b/shell/ash_test/ash-heredoc/heredoc_empty.right new file mode 100644 index 000000000..0eabe3671 --- /dev/null +++ b/shell/ash_test/ash-heredoc/heredoc_empty.right @@ -0,0 +1,3 @@ +OK +OK +OK diff --git a/shell/ash_test/ash-heredoc/heredoc_empty.tests b/shell/ash_test/ash-heredoc/heredoc_empty.tests new file mode 100755 index 000000000..3629bc6d1 --- /dev/null +++ b/shell/ash_test/ash-heredoc/heredoc_empty.tests @@ -0,0 +1,8 @@ +unset a +cat <<- $a + OK +$a +cat <<- "" + OK + +echo OK -- cgit v1.2.3-55-g6feb From 0f018b30700989462de0a15b8285206d16170c1f Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 29 Jul 2017 20:43:26 +0200 Subject: hush: fix handling of empty heredoc EOF marker function old new delta parse_stream 2609 2634 +25 Signed-off-by: Denys Vlasenko --- shell/ash_test/ash-heredoc/heredoc_empty2.right | 4 +++ shell/ash_test/ash-heredoc/heredoc_empty2.tests | 14 ++++++++ shell/hush.c | 44 ++++++++++++++--------- shell/hush_test/hush-heredoc/heredoc_empty2.right | 4 +++ shell/hush_test/hush-heredoc/heredoc_empty2.tests | 14 ++++++++ 5 files changed, 63 insertions(+), 17 deletions(-) create mode 100644 shell/ash_test/ash-heredoc/heredoc_empty2.right create mode 100755 shell/ash_test/ash-heredoc/heredoc_empty2.tests create mode 100644 shell/hush_test/hush-heredoc/heredoc_empty2.right create mode 100755 shell/hush_test/hush-heredoc/heredoc_empty2.tests (limited to 'shell') diff --git a/shell/ash_test/ash-heredoc/heredoc_empty2.right b/shell/ash_test/ash-heredoc/heredoc_empty2.right new file mode 100644 index 000000000..e32c6ea58 --- /dev/null +++ b/shell/ash_test/ash-heredoc/heredoc_empty2.right @@ -0,0 +1,4 @@ +OK1 +Ok:0 +OK2 +Ok:0 diff --git a/shell/ash_test/ash-heredoc/heredoc_empty2.tests b/shell/ash_test/ash-heredoc/heredoc_empty2.tests new file mode 100755 index 000000000..20fc35fe9 --- /dev/null +++ b/shell/ash_test/ash-heredoc/heredoc_empty2.tests @@ -0,0 +1,14 @@ +unset a + +# Heredoc with empty delimiter +cat <<- "" + OK1 + +echo Ok:$? + +# Heredoc with empty delimiter +cat <<- "" + OK2 + + +echo Ok:$? diff --git a/shell/hush.c b/shell/hush.c index d0225edb9..0fae809b3 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -4001,24 +4001,34 @@ static char *fetch_till_str(o_string *as_string, ch = i_getch(input); if (ch != EOF) nommu_addchr(as_string, ch); - if ((ch == '\n' || ch == EOF) - && ((heredoc_flags & HEREDOC_QUOTED) || prev != '\\') - ) { - if (strcmp(heredoc.data + past_EOL, word) == 0) { - heredoc.data[past_EOL] = '\0'; - debug_printf_parse("parsed heredoc '%s'\n", heredoc.data); - return heredoc.data; - } - while (ch == '\n') { - o_addchr(&heredoc, ch); - prev = ch; + if (ch == '\n' || ch == EOF) { + check_heredoc_end: + if ((heredoc_flags & HEREDOC_QUOTED) || prev != '\\') { + if (strcmp(heredoc.data + past_EOL, word) == 0) { + heredoc.data[past_EOL] = '\0'; + debug_printf_parse("parsed heredoc '%s'\n", heredoc.data); + return heredoc.data; + } + if (ch == '\n') { + /* This is a new line. + * Remember position and backslash-escaping status. + */ + o_addchr(&heredoc, ch); + prev = ch; jump_in: - past_EOL = heredoc.length; - do { - ch = i_getch(input); - if (ch != EOF) - nommu_addchr(as_string, ch); - } while ((heredoc_flags & HEREDOC_SKIPTABS) && ch == '\t'); + past_EOL = heredoc.length; + /* Get 1st char of next line, possibly skipping leading tabs */ + do { + ch = i_getch(input); + if (ch != EOF) + nommu_addchr(as_string, ch); + } while ((heredoc_flags & HEREDOC_SKIPTABS) && ch == '\t'); + /* If this immediately ended the line, + * go back to end-of-line checks. + */ + if (ch == '\n') + goto check_heredoc_end; + } } } if (ch == EOF) { diff --git a/shell/hush_test/hush-heredoc/heredoc_empty2.right b/shell/hush_test/hush-heredoc/heredoc_empty2.right new file mode 100644 index 000000000..e32c6ea58 --- /dev/null +++ b/shell/hush_test/hush-heredoc/heredoc_empty2.right @@ -0,0 +1,4 @@ +OK1 +Ok:0 +OK2 +Ok:0 diff --git a/shell/hush_test/hush-heredoc/heredoc_empty2.tests b/shell/hush_test/hush-heredoc/heredoc_empty2.tests new file mode 100755 index 000000000..20fc35fe9 --- /dev/null +++ b/shell/hush_test/hush-heredoc/heredoc_empty2.tests @@ -0,0 +1,14 @@ +unset a + +# Heredoc with empty delimiter +cat <<- "" + OK1 + +echo Ok:$? + +# Heredoc with empty delimiter +cat <<- "" + OK2 + + +echo Ok:$? -- cgit v1.2.3-55-g6feb From 469998015f57a552718420a4050d90e5dae6a6fd Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 29 Jul 2017 21:12:29 +0200 Subject: ash: [PARSER] Add FAKEEOFMARK for expandstr Upstream commit: Date: Thu, 27 Dec 2007 13:54:16 +1100 [PARSER] Add FAKEEOFMARK for expandstr Previously expandstr used the string "" to indicate that it needs to be treated just like a here-doc except that there is no terminator. However, the string "" is in fact a valid here-doc terminator so now that we deal with it correctly expandstr no longer works in the presence of new-lines in the prompt. This patch introduces the FAKEEOFMARK macro which does not equal any real EOF marker but is distinct from the NULL pointer which is used to indicate non-here-doc contexts. Thanks to Markus Triska for reporting this regression. Signed-off-by: Herbert Xu Unfortunately, I did not find the failing example for this old fix. I also tweaked the code which was added by this commit: " Date: Mon Sep 24 18:30:02 2007 +0000 ash: fix prompt expansion (Natanael Copa ) " since other parts of code do expect expandstr() to use DQSYNTAX, not PSSYNTAX. function old new delta parse_stream 2609 2634 +25 setprompt_if 128 133 +5 read_profile 32 37 +5 evalcommand 1334 1339 +5 expandstr 122 120 -2 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 4/1 up/down: 40/-2) Total: 38 bytes Signed-off-by: Denys Vlasenko --- shell/ash.c | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index 2bfec83e6..3184249f5 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -2488,8 +2488,18 @@ putprompt(const char *s) #endif /* expandstr() needs parsing machinery, so it is far away ahead... */ -static const char *expandstr(const char *ps); +static const char *expandstr(const char *ps, int syntax_type); +/* Values for syntax param */ +#define BASESYNTAX 0 /* not in quotes */ +#define DQSYNTAX 1 /* in double quotes */ +#define SQSYNTAX 2 /* in single quotes */ +#define ARISYNTAX 3 /* in arithmetic */ +#define PSSYNTAX 4 /* prompt. never passed to SIT() */ +/* PSSYNTAX expansion is identical to DQSYNTAX, except keeping '\$' as '\$' */ +/* + * called by editline -- any expansions to the prompt should be added here. + */ static void setprompt_if(smallint do_set, int whichprompt) { @@ -2513,7 +2523,7 @@ setprompt_if(smallint do_set, int whichprompt) } #if ENABLE_ASH_EXPAND_PRMT pushstackmark(&smark, stackblocksize()); - putprompt(expandstr(prompt)); + putprompt(expandstr(prompt, PSSYNTAX)); popstackmark(&smark); #else putprompt(prompt); @@ -2838,13 +2848,6 @@ enum { /* c in SIT(c, syntax) must be an *unsigned char* or PEOA or PEOF, * caller must ensure proper cast on it if c is *char_ptr! */ -/* Values for syntax param */ -#define BASESYNTAX 0 /* not in quotes */ -#define DQSYNTAX 1 /* in double quotes */ -#define SQSYNTAX 2 /* in single quotes */ -#define ARISYNTAX 3 /* in arithmetic */ -#define PSSYNTAX 4 /* prompt. never passed to SIT() */ - #if USE_SIT_FUNCTION static int @@ -9709,7 +9712,7 @@ evalcommand(union node *cmd, int flags) if (xflag) { const char *pfx = ""; - fdprintf(preverrout_fd, "%s", expandstr(ps4val())); + fdprintf(preverrout_fd, "%s", expandstr(ps4val(), DQSYNTAX)); sp = varlist.list; while (sp) { @@ -11522,6 +11525,15 @@ decode_dollar_squote(void) } #endif +/* Used by expandstr to get here-doc like behaviour. */ +#define FAKEEOFMARK ((char*)(uintptr_t)1) + +static ALWAYS_INLINE int +realeofmark(const char *eofmark) +{ + return eofmark && eofmark != FAKEEOFMARK; +} + /* * If eofmark is NULL, read a word or a redirection symbol. If eofmark * is not NULL, read a here document. In the latter case, eofmark is the @@ -11767,7 +11779,7 @@ readtoken1(int c, int syntax, char *eofmark, int striptabs) * we are at the end of the here document, this routine sets the c to PEOF. */ checkend: { - if (eofmark) { + if (realeofmark(eofmark)) { int markloc; char *p; @@ -12472,22 +12484,18 @@ parseheredoc(void) } -/* - * called by editline -- any expansions to the prompt should be added here. - */ static const char * -expandstr(const char *ps) +expandstr(const char *ps, int syntax_type) { union node n; int saveprompt; - /* XXX Fix (char *) cast. It _is_ a bug. ps is variable's value, - * and token processing _can_ alter it (delete NULs etc). */ + /* XXX Fix (char *) cast. */ setinputstring((char *)ps); saveprompt = doprompt; doprompt = 0; - readtoken1(pgetc(), PSSYNTAX, nullstr, 0); + readtoken1(pgetc(), syntax_type, FAKEEOFMARK, 0); doprompt = saveprompt; popfile(); @@ -13548,7 +13556,7 @@ procargs(char **argv) static void read_profile(const char *name) { - name = expandstr(name); + name = expandstr(name, DQSYNTAX); if (setinputfile(name, INPUT_PUSH_FILE | INPUT_NOFILE_OK) < 0) return; cmdloop(0); -- cgit v1.2.3-55-g6feb From 1c79aeb6a8cae4cd4013926b97f39305b689d74c Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 29 Jul 2017 22:51:52 +0200 Subject: ash: [REDIR] Fix popredir on abnormal exit from built-in Upstream commit: Date: Thu, 27 May 2010 15:03:46 +0800 [REDIR] Fix popredir on abnormal exit from built-in Just like the poplocalvar problem recently fixed, redirections can also be leaked in case of an abnormal exit. This patch fixes it using the same method as poplocalvar, by storing the previous redirection state and restoring to that point. Signed-off-by: Herbert Xu Signed-off-by: Denys Vlasenko --- shell/ash.c | 70 ++++++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 44 insertions(+), 26 deletions(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index 3184249f5..e2b4eee95 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -5435,33 +5435,13 @@ redirect(union node *redir, int flags) int newfd; int copied_fd2 = -1; - if (!redir) { + if (!redir) return; - } - sv = NULL; - sv_pos = 0; INT_OFF; - if (flags & REDIR_PUSH) { - union node *tmp = redir; - do { - sv_pos++; -#if BASH_REDIR_OUTPUT - if (tmp->nfile.type == NTO2) - sv_pos++; -#endif - tmp = tmp->nfile.next; - } while (tmp); - sv = ckmalloc(sizeof(*sv) + sv_pos * sizeof(sv->two_fd[0])); - sv->next = redirlist; - sv->pair_count = sv_pos; - redirlist = sv; - while (sv_pos > 0) { - sv_pos--; - sv->two_fd[sv_pos].orig = sv->two_fd[sv_pos].copy = EMPTY; - } - } - + if (flags & REDIR_PUSH) + sv = redirlist; + sv_pos = 0; do { int right_fd = -1; fd = redir->nfile.fd; @@ -5583,6 +5563,34 @@ redirectsafe(union node *redir, int flags) return err; } +static struct redirtab* +pushredir(union node *redir) +{ + struct redirtab *sv; + int i; + + if (!redir) + return redirlist; + + i = 0; + do { + i++; +#if BASH_REDIR_OUTPUT + if (redir->nfile.type == NTO2) + i++; +#endif + redir = redir->nfile.next; + } while (redir); + + sv = ckzalloc(sizeof(*sv) + i * sizeof(sv->two_fd[0])); + sv->pair_count = i; + while (--i >= 0) + sv->two_fd[i].orig = sv->two_fd[i].copy = EMPTY; + sv->next = redirlist; + redirlist = sv; + return sv->next; +} + /* * Undo the effects of the last redirection. */ @@ -5618,6 +5626,13 @@ popredir(int drop, int restore) INT_ON; } +static void +unwindredir(struct redirtab *stop) +{ + while (redirlist != stop) + popredir(/*drop:*/ 0, /*restore:*/ 0); +} + /* ============ Routines to expand arguments to commands * @@ -8727,6 +8742,7 @@ evaltree(union node *n, int flags) goto setstatus; case NREDIR: expredir(n->nredir.redirect); + pushredir(n->nredir.redirect); status = redirectsafe(n->nredir.redirect, REDIR_PUSH); if (!status) { status = evaltree(n->nredir.n, flags & EV_TESTED); @@ -9622,6 +9638,7 @@ evalcommand(union node *cmd, int flags) "\0\0", bltincmd /* why three NULs? */ }; struct localvar_list *localvar_stop; + struct redirtab *redir_stop; struct stackmark smark; union node *argp; struct arglist arglist; @@ -9687,6 +9704,7 @@ evalcommand(union node *cmd, int flags) preverrout_fd = 2; expredir(cmd->ncmd.redirect); + redir_stop = pushredir(cmd->ncmd.redirect); status = redirectsafe(cmd->ncmd.redirect, REDIR_PUSH | REDIR_SAVEFD2); path = vpath.var_text; @@ -9878,6 +9896,7 @@ evalcommand(union node *cmd, int flags) out: if (cmd->ncmd.redirect) popredir(/*drop:*/ cmd_is_exec, /*restore:*/ cmd_is_exec); + unwindredir(redir_stop); unwindlocalvars(localvar_stop); if (lastarg) { /* dsl: I think this is intended to be used to support @@ -13584,8 +13603,7 @@ reset(void) popallfiles(); /* from redir.c: */ - while (redirlist) - popredir(/*drop:*/ 0, /*restore:*/ 0); + unwindredir(NULL); /* from var.c: */ unwindlocalvars(NULL); -- cgit v1.2.3-55-g6feb From 5f0a75f24b5afdedf5b67a7f42184ed196e1a5c9 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 29 Jul 2017 22:58:44 +0200 Subject: ash: if !ENABLE_ASH_EXPAND_PRMT, disable PSSYNTAX code Signed-off-by: Denys Vlasenko --- shell/ash.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index e2b4eee95..52fcc7944 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -2494,7 +2494,9 @@ static const char *expandstr(const char *ps, int syntax_type); #define DQSYNTAX 1 /* in double quotes */ #define SQSYNTAX 2 /* in single quotes */ #define ARISYNTAX 3 /* in arithmetic */ -#define PSSYNTAX 4 /* prompt. never passed to SIT() */ +#if ENABLE_ASH_EXPAND_PRMT +# define PSSYNTAX 4 /* prompt. never passed to SIT() */ +#endif /* PSSYNTAX expansion is identical to DQSYNTAX, except keeping '\$' as '\$' */ /* @@ -11594,9 +11596,13 @@ readtoken1(int c, int syntax, char *eofmark, int striptabs) bqlist = NULL; quotef = 0; IF_FEATURE_SH_MATH(prevsyntax = 0;) +#if ENABLE_ASH_EXPAND_PRMT pssyntax = (syntax == PSSYNTAX); if (pssyntax) syntax = DQSYNTAX; +#else + pssyntax = 0; /* constant */ +#endif dblquote = (syntax == DQSYNTAX); varnest = 0; IF_FEATURE_SH_MATH(arinest = 0;) @@ -11650,7 +11656,7 @@ readtoken1(int c, int syntax, char *eofmark, int striptabs) } else if (c == '\n') { nlprompt(); } else { - if (c == '$' && pssyntax) { + if (pssyntax && c == '$') { USTPUTC(CTLESC, out); USTPUTC('\\', out); } -- cgit v1.2.3-55-g6feb From d07a15bd1ba99caa9386234cda1e8c83897c7553 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sun, 30 Jul 2017 16:51:05 +0200 Subject: ash: remove REDIR_SAVEFD2 function old new delta evalcommand 1364 1369 +5 redirect 1055 1014 -41 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 1/1 up/down: 5/-41) Total: -36 bytes Signed-off-by: Denys Vlasenko --- shell/ash.c | 69 +++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 37 insertions(+), 32 deletions(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index 52fcc7944..1e720aec4 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -2030,7 +2030,7 @@ struct redirtab; struct globals_var { struct shparam shellparam; /* $@ current positional parameters */ struct redirtab *redirlist; - int preverrout_fd; /* save fd2 before print debug if xflag is set. */ + int preverrout_fd; /* stderr fd: usually 2, unless redirect moved it */ struct var *vartab[VTABSIZE]; struct var varinit[ARRAY_SIZE(varinit_data)]; }; @@ -5426,16 +5426,13 @@ is_hidden_fd(struct redirtab *rp, int fd) */ /* flags passed to redirect */ #define REDIR_PUSH 01 /* save previous values of file descriptors */ -#define REDIR_SAVEFD2 03 /* set preverrout */ static void redirect(union node *redir, int flags) { struct redirtab *sv; int sv_pos; - int i; int fd; int newfd; - int copied_fd2 = -1; if (!redir) return; @@ -5479,37 +5476,38 @@ redirect(union node *redir, int flags) * to the same fd as right side fd in N>&M */ int minfd = right_fd < 10 ? 10 : right_fd + 1; #if defined(F_DUPFD_CLOEXEC) - i = fcntl(fd, F_DUPFD_CLOEXEC, minfd); + int copy = fcntl(fd, F_DUPFD_CLOEXEC, minfd); #else - i = fcntl(fd, F_DUPFD, minfd); + int copy = fcntl(fd, F_DUPFD, minfd); #endif - if (i == -1) { - i = errno; - if (i != EBADF) { + if (copy == -1) { + int e = errno; + if (e != EBADF) { /* Strange error (e.g. "too many files" EMFILE?) */ if (newfd >= 0) close(newfd); - errno = i; + errno = e; ash_msg_and_raise_perror("%d", fd); /* NOTREACHED */ } /* EBADF: it is not open - good, remember to close it */ remember_to_close: - i = CLOSED; + copy = CLOSED; } else { /* fd is open, save its copy */ #if !defined(F_DUPFD_CLOEXEC) - fcntl(i, F_SETFD, FD_CLOEXEC); + fcntl(copy, F_SETFD, FD_CLOEXEC); #endif /* "exec fd>&-" should not close fds * which point to script file(s). * Force them to be restored afterwards */ if (is_hidden_fd(sv, fd)) - i |= COPYFD_RESTORE; + copy |= COPYFD_RESTORE; } - if (fd == 2) - copied_fd2 = i; + /* if we move stderr, let "set -x" code know */ + if (fd == preverrout_fd) + preverrout_fd = copy; sv->two_fd[sv_pos].orig = fd; - sv->two_fd[sv_pos].copy = i; + sv->two_fd[sv_pos].copy = copy; sv_pos++; } if (newfd < 0) { @@ -5537,10 +5535,16 @@ redirect(union node *redir, int flags) } #endif } while ((redir = redir->nfile.next) != NULL); - INT_ON; - if ((flags & REDIR_SAVEFD2) && copied_fd2 >= 0) - preverrout_fd = copied_fd2; + +//dash:#define REDIR_SAVEFD2 03 /* set preverrout */ +#define REDIR_SAVEFD2 0 + // dash has a bug: since REDIR_SAVEFD2=3 and REDIR_PUSH=1, this test + // triggers for pure REDIR_PUSH too. Thus, this is done almost always, + // not only for calls with flags containing REDIR_SAVEFD2. + // We do this unconditionally (see code above). + //if ((flags & REDIR_SAVEFD2) && copied_fd2 >= 0) + // preverrout_fd = copied_fd2; } static int @@ -9655,9 +9659,7 @@ evalcommand(union node *cmd, int flags) int spclbltin; int status; char **nargv; - struct builtincmd *bcmd; smallint cmd_is_exec; - smallint pseudovarflag = 0; /* First expand the arguments. */ TRACE(("evalcommand(0x%lx, %d) called\n", (long)cmd, flags)); @@ -9674,21 +9676,24 @@ evalcommand(union node *cmd, int flags) argc = 0; if (cmd->ncmd.args) { + struct builtincmd *bcmd; + smallint pseudovarflag; + bcmd = find_builtin(cmd->ncmd.args->narg.text); pseudovarflag = bcmd && IS_BUILTIN_ASSIGN(bcmd); - } - for (argp = cmd->ncmd.args; argp; argp = argp->narg.next) { - struct strlist **spp; + for (argp = cmd->ncmd.args; argp; argp = argp->narg.next) { + struct strlist **spp; - spp = arglist.lastp; - if (pseudovarflag && isassignment(argp->narg.text)) - expandarg(argp, &arglist, EXP_VARTILDE); - else - expandarg(argp, &arglist, EXP_FULL | EXP_TILDE); + spp = arglist.lastp; + if (pseudovarflag && isassignment(argp->narg.text)) + expandarg(argp, &arglist, EXP_VARTILDE); + else + expandarg(argp, &arglist, EXP_FULL | EXP_TILDE); - for (sp = *spp; sp; sp = sp->next) - argc++; + for (sp = *spp; sp; sp = sp->next) + argc++; + } } /* Reserve one extra spot at the front for shellexec. */ @@ -9704,9 +9709,9 @@ evalcommand(union node *cmd, int flags) if (iflag && funcnest == 0 && argc > 0) lastarg = nargv[-1]; - preverrout_fd = 2; expredir(cmd->ncmd.redirect); redir_stop = pushredir(cmd->ncmd.redirect); + preverrout_fd = 2; status = redirectsafe(cmd->ncmd.redirect, REDIR_PUSH | REDIR_SAVEFD2); path = vpath.var_text; -- cgit v1.2.3-55-g6feb From 657e9005a9e31e1da094b260abaa8f335e92301f Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sun, 30 Jul 2017 23:34:04 +0200 Subject: hush: massage redirect code to be slightly more like ash function old new delta save_fd_on_redirect - 203 +203 xdup_CLOEXEC_and_close - 75 +75 setup_redirects 245 250 +5 xdup_and_close 72 - -72 save_fds_on_redirect 221 - -221 ------------------------------------------------------------------------------ (add/remove: 2/2 grow/shrink: 1/0 up/down: 283/-293) Total: -10 bytes Signed-off-by: Denys Vlasenko --- shell/hush.c | 109 +++++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 72 insertions(+), 37 deletions(-) (limited to 'shell') diff --git a/shell/hush.c b/shell/hush.c index 0fae809b3..d4101d66f 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -1454,11 +1454,11 @@ static int fcntl_F_DUPFD(int fd, int avoid_fd) return newfd; } -static int xdup_and_close(int fd, int F_DUPFD_maybe_CLOEXEC, int avoid_fd) +static int xdup_CLOEXEC_and_close(int fd, int avoid_fd) { int newfd; repeat: - newfd = fcntl(fd, F_DUPFD_maybe_CLOEXEC, avoid_fd + 1); + newfd = fcntl(fd, F_DUPFD_CLOEXEC, avoid_fd + 1); if (newfd < 0) { if (errno == EBUSY) goto repeat; @@ -1469,6 +1469,8 @@ static int xdup_and_close(int fd, int F_DUPFD_maybe_CLOEXEC, int avoid_fd) return fd; xfunc_die(); } + if (F_DUPFD_CLOEXEC == F_DUPFD) /* if old libc (w/o F_DUPFD_CLOEXEC) */ + fcntl(newfd, F_SETFD, FD_CLOEXEC); close(fd); return newfd; } @@ -1507,7 +1509,7 @@ static int save_FILEs_on_redirect(int fd, int avoid_fd) while (fl) { if (fd == fl->fd) { /* We use it only on script files, they are all CLOEXEC */ - fl->fd = xdup_and_close(fd, F_DUPFD_CLOEXEC, avoid_fd); + fl->fd = xdup_CLOEXEC_and_close(fd, avoid_fd); debug_printf_redir("redirect_fd %d: matches a script fd, moving it to %d\n", fd, fl->fd); return 1; } @@ -6711,18 +6713,42 @@ static struct squirrel *add_squirrel(struct squirrel *sq, int fd, int avoid_fd) return append_squirrel(sq, i, fd, moved_to); } +static struct squirrel *add_squirrel_closed(struct squirrel *sq, int fd) +{ + int i; + + i = 0; + if (sq) while (sq[i].orig_fd >= 0) { + /* If we collide with an already moved fd... */ + if (fd == sq[i].orig_fd) { + /* Examples: + * "echo 3>FILE 3>&- 3>FILE" + * "echo 3>&- 3>FILE" + * No need for last redirect to insert + * another "need to close 3" indicator. + */ + debug_printf_redir("redirect_fd %d: already moved or closed\n", fd); + return sq; + } + i++; + } + + debug_printf_redir("redirect_fd %d: previous fd was closed\n", fd); + return append_squirrel(sq, i, fd, -1); +} + /* fd: redirect wants this fd to be used (e.g. 3>file). * Move all conflicting internally used fds, * and remember them so that we can restore them later. */ -static int save_fds_on_redirect(int fd, int avoid_fd, struct squirrel **sqp) +static int save_fd_on_redirect(int fd, int avoid_fd, struct squirrel **sqp) { if (avoid_fd < 9) /* the important case here is that it can be -1 */ avoid_fd = 9; #if ENABLE_HUSH_INTERACTIVE if (fd != 0 && fd == G.interactive_fd) { - G.interactive_fd = xdup_and_close(G.interactive_fd, F_DUPFD_CLOEXEC, avoid_fd); + G.interactive_fd = xdup_CLOEXEC_and_close(G.interactive_fd, avoid_fd); debug_printf_redir("redirect_fd %d: matches interactive_fd, moving it to %d\n", fd, G.interactive_fd); return 1; /* "we closed fd" */ } @@ -6748,7 +6774,6 @@ static int save_fds_on_redirect(int fd, int avoid_fd, struct squirrel **sqp) static void restore_redirects(struct squirrel *sq) { - if (sq) { int i = 0; while (sq[i].orig_fd >= 0) { @@ -6775,13 +6800,15 @@ static void restore_redirects(struct squirrel *sq) * and stderr if they are redirected. */ static int setup_redirects(struct command *prog, struct squirrel **sqp) { - int openfd, mode; struct redir_struct *redir; for (redir = prog->redirects; redir; redir = redir->next) { + int newfd; + int closed; + if (redir->rd_type == REDIRECT_HEREDOC2) { /* "rd_fd<rd_fd, /*avoid:*/ 0, sqp); + save_fd_on_redirect(redir->rd_fd, /*avoid:*/ 0, sqp); /* for REDIRECT_HEREDOC2, rd_filename holds _contents_ * of the heredoc */ debug_printf_parse("set heredoc '%s'\n", @@ -6793,6 +6820,8 @@ static int setup_redirects(struct command *prog, struct squirrel **sqp) if (redir->rd_dup == REDIRFD_TO_FILE) { /* "rd_fd<*>file" case (<*> is <,>,>>,<>) */ char *p; + int mode; + if (redir->rd_filename == NULL) { /* * Examples: @@ -6804,9 +6833,9 @@ static int setup_redirects(struct command *prog, struct squirrel **sqp) } mode = redir_table[redir->rd_type].mode; p = expand_string_to_string(redir->rd_filename, /*unbackslash:*/ 1); - openfd = open_or_warn(p, mode); + newfd = open_or_warn(p, mode); free(p); - if (openfd < 0) { + if (newfd < 0) { /* Error message from open_or_warn can be lost * if stderr has been redirected, but bash * and ash both lose it as well @@ -6814,40 +6843,46 @@ static int setup_redirects(struct command *prog, struct squirrel **sqp) */ return 1; } - if (openfd == redir->rd_fd && sqp) { + if (newfd == redir->rd_fd && sqp) { /* open() gave us precisely the fd we wanted. * This means that this fd was not busy * (not opened to anywhere). * Remember to close it on restore: */ - struct squirrel *sq = *sqp; - int i = 0; - if (sq) while (sq[i].orig_fd >= 0) - i++; - *sqp = append_squirrel(sq, i, openfd, -1); /* -1 = "it was closed" */ - debug_printf_redir("redir to previously closed fd %d\n", openfd); + *sqp = add_squirrel_closed(*sqp, newfd); + debug_printf_redir("redir to previously closed fd %d\n", newfd); } } else { - /* "rd_fd<*>rd_dup" or "rd_fd<*>-" cases */ - openfd = redir->rd_dup; - } - - if (openfd != redir->rd_fd) { - int closed = save_fds_on_redirect(redir->rd_fd, /*avoid:*/ openfd, sqp); - if (openfd == REDIRFD_CLOSE) { - /* "rd_fd >&-" means "close me" */ - if (!closed) { - /* ^^^ optimization: saving may already - * have closed it. If not... */ - close(redir->rd_fd); - } - } else { - xdup2(openfd, redir->rd_fd); - if (redir->rd_dup == REDIRFD_TO_FILE) - /* "rd_fd > FILE" */ - close(openfd); - /* else: "rd_fd > rd_dup" */ - } + /* "rd_fd>&rd_dup" or "rd_fd>&-" case */ + newfd = redir->rd_dup; + } + + if (newfd == redir->rd_fd) + continue; + + /* if "N>FILE": move newfd to redir->rd_fd */ + /* if "N>&M": dup newfd to redir->rd_fd */ + /* if "N>&-": close redir->rd_fd (newfd is REDIRFD_CLOSE) */ + + closed = save_fd_on_redirect(redir->rd_fd, /*avoid:*/ newfd, sqp); + if (newfd == REDIRFD_CLOSE) { + /* "N>&-" means "close me" */ + if (!closed) { + /* ^^^ optimization: saving may already + * have closed it. If not... */ + close(redir->rd_fd); + } + /* Sometimes we do another close on restore, getting EBADF. + * Consider "echo 3>FILE 3>&-" + * first redirect remembers "need to close 3", + * and second redirect closes 3! Restore code then closes 3 again. + */ + } else { + xdup2(newfd, redir->rd_fd); + if (redir->rd_dup == REDIRFD_TO_FILE) + /* "rd_fd > FILE" */ + close(newfd); + /* else: "rd_fd > rd_dup" */ } } return 0; -- cgit v1.2.3-55-g6feb From 035486c7500c09616a6c1040d8e70923532a5c2d Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 31 Jul 2017 04:09:19 +0200 Subject: ash: significant overhaul of redirect saving logic New code is similar to what hush is doing. Make CLOSED to -1: same as dash. popredir() loses "restore" parameter: same as dash. COPYFD_RESTORE bit is no longer necessary. This change fixes this interactive bug: $ ls -l /proc/$$/fd 10>&- ash: can't set tty process group: Bad file descriptor ash: can't set tty process group: Bad file descriptor [1]+ Done(2) ls -l /proc/${\$}/fd 10>&4294967295 function old new delta unwindredir 29 27 -2 tryexec 154 152 -2 evaltree 503 501 -2 evalcommand 1369 1367 -2 cmdloop 187 185 -2 redirect 1029 1018 -11 popredir 153 123 -30 need_to_remember 36 - -36 is_hidden_fd 68 - -68 ------------------------------------------------------------------------------ (add/remove: 0/2 grow/shrink: 0/7 up/down: 0/-155) Total: -155 bytes text data bss dec hex filename 914572 485 6848 921905 e1131 busybox_old 914553 485 6848 921886 e111e busybox_unstripped Signed-off-by: Denys Vlasenko --- shell/ash.c | 332 ++++++++++++--------- shell/ash_test/ash-redir/redir_script.tests | 9 +- shell/ash_test/ash-redir/redir_to_bad_fd255.right | 2 + shell/ash_test/ash-redir/redir_to_bad_fd255.tests | 3 + shell/ash_test/ash-redir/redir_to_bad_fd3.right | 2 + shell/ash_test/ash-redir/redir_to_bad_fd3.tests | 3 + shell/hush_test/hush-redir/redir_script.tests | 9 +- .../hush_test/hush-redir/redir_to_bad_fd255.right | 1 + .../hush_test/hush-redir/redir_to_bad_fd255.tests | 3 + shell/hush_test/hush-redir/redir_to_bad_fd3.right | 1 + shell/hush_test/hush-redir/redir_to_bad_fd3.tests | 3 + 11 files changed, 225 insertions(+), 143 deletions(-) create mode 100644 shell/ash_test/ash-redir/redir_to_bad_fd255.right create mode 100755 shell/ash_test/ash-redir/redir_to_bad_fd255.tests create mode 100644 shell/ash_test/ash-redir/redir_to_bad_fd3.right create mode 100755 shell/ash_test/ash-redir/redir_to_bad_fd3.tests create mode 100644 shell/hush_test/hush-redir/redir_to_bad_fd255.right create mode 100755 shell/hush_test/hush-redir/redir_to_bad_fd255.tests create mode 100644 shell/hush_test/hush-redir/redir_to_bad_fd3.right create mode 100755 shell/hush_test/hush-redir/redir_to_bad_fd3.tests (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index 1e720aec4..ac676c2dc 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -5192,7 +5192,7 @@ stoppedjobs(void) #undef EMPTY #undef CLOSED #define EMPTY -2 /* marks an unused slot in redirtab */ -#define CLOSED -3 /* marks a slot of previously-closed fd */ +#define CLOSED -1 /* marks a slot of previously-closed fd */ /* * Handle here documents. Normally we fork off a process to write the @@ -5349,74 +5349,158 @@ dup2_or_raise(int from, int to) } return newfd; } +static int +fcntl_F_DUPFD(int fd, int avoid_fd) +{ + int newfd; + repeat: + newfd = fcntl(fd, F_DUPFD, avoid_fd + 1); + if (newfd < 0) { + if (errno == EBUSY) + goto repeat; + if (errno == EINTR) + goto repeat; + } + return newfd; +} +static int +xdup_CLOEXEC_and_close(int fd, int avoid_fd) +{ + int newfd; + repeat: + newfd = fcntl(fd, F_DUPFD, avoid_fd + 1); + if (newfd < 0) { + if (errno == EBUSY) + goto repeat; + if (errno == EINTR) + goto repeat; + /* fd was not open? */ + if (errno == EBADF) + return fd; + ash_msg_and_raise_perror("%d", newfd); + } + fcntl(newfd, F_SETFD, FD_CLOEXEC); + close(fd); + return newfd; +} /* Struct def and variable are moved down to the first usage site */ -struct two_fd_t { - int orig, copy; +struct squirrel { + int orig_fd; + int moved_to; }; struct redirtab { struct redirtab *next; int pair_count; - struct two_fd_t two_fd[]; + struct squirrel two_fd[]; }; #define redirlist (G_var.redirlist) -enum { - COPYFD_RESTORE = (int)~(INT_MAX), -}; -static int -need_to_remember(struct redirtab *rp, int fd) +static void +add_squirrel_closed(struct redirtab *sq, int fd) { int i; - if (!rp) /* remembering was not requested */ - return 0; + if (!sq) + return; - for (i = 0; i < rp->pair_count; i++) { - if (rp->two_fd[i].orig == fd) { - /* already remembered */ - return 0; + for (i = 0; sq->two_fd[i].orig_fd != EMPTY; i++) { + /* If we collide with an already moved fd... */ + if (fd == sq->two_fd[i].orig_fd) { + /* Examples: + * "echo 3>FILE 3>&- 3>FILE" + * "echo 3>&- 3>FILE" + * No need for last redirect to insert + * another "need to close 3" indicator. + */ + TRACE(("redirect_fd %d: already moved or closed\n", fd)); + return; } } - return 1; + TRACE(("redirect_fd %d: previous fd was closed\n", fd)); + sq->two_fd[i].orig_fd = fd; + sq->two_fd[i].moved_to = CLOSED; } -/* "hidden" fd is a fd used to read scripts, or a copy of such */ static int -is_hidden_fd(struct redirtab *rp, int fd) +save_fd_on_redirect(int fd, int avoid_fd, struct redirtab *sq) { - int i; - struct parsefile *pf; + int i, new_fd; + + if (avoid_fd < 9) /* the important case here is that it can be -1 */ + avoid_fd = 9; - if (fd == -1) +#if JOBS + if (fd == ttyfd) { + /* Testcase: "ls -l /proc/$$/fd 10>&-" should work */ + ttyfd = xdup_CLOEXEC_and_close(ttyfd, avoid_fd); + TRACE(("redirect_fd %d: matches ttyfd, moving it to %d\n", fd, ttyfd)); + return 1; /* "we closed fd" */ + } +#endif + /* Are we called from redirect(0)? E.g. redirect + * in a forked child. No need to save fds, + * we aren't going to use them anymore, ok to trash. + */ + if (!sq) return 0; - /* Check open scripts' fds */ - pf = g_parsefile; - while (pf) { - /* We skip pf_fd == 0 case because of the following case: - * $ ash # running ash interactively - * $ . ./script.sh - * and in script.sh: "exec 9>&0". - * Even though top-level pf_fd _is_ 0, - * it's still ok to use it: "read" builtin uses it, - * why should we cripple "exec" builtin? - */ - if (pf->pf_fd > 0 && fd == pf->pf_fd) { - return 1; + + /* If this one of script's fds? */ + if (fd != 0) { + struct parsefile *pf = g_parsefile; + while (pf) { + /* We skip fd == 0 case because of the following: + * $ ash # running ash interactively + * $ . ./script.sh + * and in script.sh: "exec 9>&0". + * Even though top-level pf_fd _is_ 0, + * it's still ok to use it: "read" builtin uses it, + * why should we cripple "exec" builtin? + */ + if (fd == pf->pf_fd) { + pf->pf_fd = xdup_CLOEXEC_and_close(fd, avoid_fd); + return 1; /* "we closed fd" */ + } + pf = pf->prev; } - pf = pf->prev; } - if (!rp) - return 0; - /* Check saved fds of redirects */ - fd |= COPYFD_RESTORE; - for (i = 0; i < rp->pair_count; i++) { - if (rp->two_fd[i].copy == fd) { - return 1; + /* Check whether it collides with any open fds (e.g. stdio), save fds as needed */ + + /* First: do we collide with some already moved fds? */ + for (i = 0; sq->two_fd[i].orig_fd != EMPTY; i++) { + /* If we collide with an already moved fd... */ + if (fd == sq->two_fd[i].moved_to) { + new_fd = fcntl_F_DUPFD(fd, avoid_fd); + sq->two_fd[i].moved_to = new_fd; + TRACE(("redirect_fd %d: already busy, moving to %d\n", fd, new_fd)); + if (new_fd < 0) /* what? */ + xfunc_die(); + return 0; /* "we did not close fd" */ + } + if (fd == sq->two_fd[i].orig_fd) { + /* Example: echo Hello >/dev/null 1>&2 */ + TRACE(("redirect_fd %d: already moved\n", fd)); + return 0; /* "we did not close fd" */ } } - return 0; + + /* If this fd is open, we move and remember it; if it's closed, new_fd = CLOSED (-1) */ + new_fd = fcntl_F_DUPFD(fd, avoid_fd); + TRACE(("redirect_fd %d: previous fd is moved to %d (-1 if it was closed)\n", fd, new_fd)); + if (new_fd < 0) { + if (errno != EBADF) + xfunc_die(); + /* new_fd = CLOSED; - already is -1 */ + } + sq->two_fd[i].moved_to = new_fd; + sq->two_fd[i].orig_fd = fd; + + /* if we move stderr, let "set -x" code know */ + if (fd == preverrout_fd) + preverrout_fd = new_fd; + + return 0; /* "we did not close fd" */ } /* @@ -5431,109 +5515,80 @@ redirect(union node *redir, int flags) { struct redirtab *sv; int sv_pos; - int fd; - int newfd; if (!redir) return; + + sv_pos = 0; sv = NULL; INT_OFF; if (flags & REDIR_PUSH) sv = redirlist; - sv_pos = 0; do { - int right_fd = -1; + int fd; + int newfd; + int close_fd; + int closed; + fd = redir->nfile.fd; if (redir->nfile.type == NTOFD || redir->nfile.type == NFROMFD) { - right_fd = redir->ndup.dupfd; - //bb_error_msg("doing %d > %d", fd, right_fd); - /* redirect from/to same file descriptor? */ - if (right_fd == fd) - continue; - /* "echo >&10" and 10 is a fd opened to a sh script? */ - if (is_hidden_fd(sv, right_fd)) { - errno = EBADF; /* as if it is closed */ - ash_msg_and_raise_perror("%d", right_fd); - } - newfd = -1; + //bb_error_msg("doing %d > %d", fd, newfd); + newfd = redir->ndup.dupfd; + close_fd = -1; } else { newfd = openredirect(redir); /* always >= 0 */ if (fd == newfd) { - /* Descriptor wasn't open before redirect. - * Mark it for close in the future */ - if (need_to_remember(sv, fd)) { - goto remember_to_close; - } + /* open() gave us precisely the fd we wanted. + * This means that this fd was not busy + * (not opened to anywhere). + * Remember to close it on restore: + */ + add_squirrel_closed(sv, fd); continue; } + close_fd = newfd; } -#if BASH_REDIR_OUTPUT - redirect_more: -#endif - if (need_to_remember(sv, fd)) { - /* Copy old descriptor */ - /* Careful to not accidentally "save" - * to the same fd as right side fd in N>&M */ - int minfd = right_fd < 10 ? 10 : right_fd + 1; -#if defined(F_DUPFD_CLOEXEC) - int copy = fcntl(fd, F_DUPFD_CLOEXEC, minfd); -#else - int copy = fcntl(fd, F_DUPFD, minfd); -#endif - if (copy == -1) { - int e = errno; - if (e != EBADF) { - /* Strange error (e.g. "too many files" EMFILE?) */ - if (newfd >= 0) - close(newfd); - errno = e; - ash_msg_and_raise_perror("%d", fd); - /* NOTREACHED */ - } - /* EBADF: it is not open - good, remember to close it */ - remember_to_close: - copy = CLOSED; - } else { /* fd is open, save its copy */ -#if !defined(F_DUPFD_CLOEXEC) - fcntl(copy, F_SETFD, FD_CLOEXEC); -#endif - /* "exec fd>&-" should not close fds - * which point to script file(s). - * Force them to be restored afterwards */ - if (is_hidden_fd(sv, fd)) - copy |= COPYFD_RESTORE; - } - /* if we move stderr, let "set -x" code know */ - if (fd == preverrout_fd) - preverrout_fd = copy; - sv->two_fd[sv_pos].orig = fd; - sv->two_fd[sv_pos].copy = copy; - sv_pos++; - } - if (newfd < 0) { - /* NTOFD/NFROMFD: copy redir->ndup.dupfd to fd */ - if (redir->ndup.dupfd < 0) { /* "fd>&-" */ - /* Don't want to trigger debugging */ - if (fd != -1) - close(fd); - } else { - dup2_or_raise(redir->ndup.dupfd, fd); + + if (fd == newfd) + continue; + + /* if "N>FILE": move newfd to fd */ + /* if "N>&M": dup newfd to fd */ + /* if "N>&-": close fd (newfd is -1) */ + + IF_BASH_REDIR_OUTPUT(redirect_more:) + + closed = save_fd_on_redirect(fd, /*avoid:*/ newfd, sv); + if (newfd == -1) { + /* "N>&-" means "close me" */ + if (!closed) { + /* ^^^ optimization: saving may already + * have closed it. If not... */ + close(fd); } - } else if (fd != newfd) { /* move newfd to fd */ + } else { +///TODO: if _newfd_ is a script fd or saved fd, then simulate EBADF! +//if (newfd == ttyfd) { +// errno = EBADF; +// ash_msg_and_raise_perror("A %d", newfd); +//} +//if (newfd == g_parsefile->pf_fd) { +// errno = EBADF; +// ash_msg_and_raise_perror("B %d", newfd); +//} dup2_or_raise(newfd, fd); + if (close_fd >= 0) /* "N>FILE" or ">&FILE" or heredoc? */ + close(close_fd); #if BASH_REDIR_OUTPUT - if (!(redir->nfile.type == NTO2 && fd == 2)) + if (redir->nfile.type == NTO2 && fd == 1) { + /* ">&FILE". we already redirected to 1, now copy 1 to 2 */ + fd = 2; + newfd = 1; + close_fd = -1; + goto redirect_more; + } #endif - close(newfd); } -#if BASH_REDIR_OUTPUT - if (redir->nfile.type == NTO2 && fd == 1) { - /* We already redirected it to fd 1, now copy it to 2 */ - newfd = 1; - fd = 2; - goto redirect_more; - } -#endif } while ((redir = redir->nfile.next) != NULL); INT_ON; @@ -5542,7 +5597,7 @@ redirect(union node *redir, int flags) // dash has a bug: since REDIR_SAVEFD2=3 and REDIR_PUSH=1, this test // triggers for pure REDIR_PUSH too. Thus, this is done almost always, // not only for calls with flags containing REDIR_SAVEFD2. - // We do this unconditionally (see code above). + // We do this unconditionally (see save_fd_on_redirect()). //if ((flags & REDIR_SAVEFD2) && copied_fd2 >= 0) // preverrout_fd = copied_fd2; } @@ -5557,7 +5612,7 @@ redirectsafe(union node *redir, int flags) SAVE_INT(saveint); /* "echo 9>/dev/null; echo >&9; echo result: $?" - result should be 1, not 2! */ - err = setjmp(jmploc.loc); // huh?? was = setjmp(jmploc.loc) * 2; + err = setjmp(jmploc.loc); /* was = setjmp(jmploc.loc) * 2; */ if (!err) { exception_handler = &jmploc; redirect(redir, flags); @@ -5591,7 +5646,7 @@ pushredir(union node *redir) sv = ckzalloc(sizeof(*sv) + i * sizeof(sv->two_fd[0])); sv->pair_count = i; while (--i >= 0) - sv->two_fd[i].orig = sv->two_fd[i].copy = EMPTY; + sv->two_fd[i].orig_fd = sv->two_fd[i].moved_to = EMPTY; sv->next = redirlist; redirlist = sv; return sv->next; @@ -5601,7 +5656,7 @@ pushredir(union node *redir) * Undo the effects of the last redirection. */ static void -popredir(int drop, int restore) +popredir(int drop) { struct redirtab *rp; int i; @@ -5611,20 +5666,19 @@ popredir(int drop, int restore) INT_OFF; rp = redirlist; for (i = 0; i < rp->pair_count; i++) { - int fd = rp->two_fd[i].orig; - int copy = rp->two_fd[i].copy; + int fd = rp->two_fd[i].orig_fd; + int copy = rp->two_fd[i].moved_to; if (copy == CLOSED) { if (!drop) close(fd); continue; } if (copy != EMPTY) { - if (!drop || (restore && (copy & COPYFD_RESTORE))) { - copy &= ~COPYFD_RESTORE; + if (!drop) { /*close(fd);*/ dup2_or_raise(copy, fd); } - close(copy & ~COPYFD_RESTORE); + close(copy); } } redirlist = rp->next; @@ -5636,7 +5690,7 @@ static void unwindredir(struct redirtab *stop) { while (redirlist != stop) - popredir(/*drop:*/ 0, /*restore:*/ 0); + popredir(/*drop:*/ 0); } @@ -7715,7 +7769,7 @@ tryexec(IF_FEATURE_SH_STANDALONE(int applet_no,) const char *cmd, char **argv, c clearenv(); while (*envp) putenv(*envp++); - popredir(/*drop:*/ 1, /*restore:*/ 0); + popredir(/*drop:*/ 1); run_applet_no_and_exit(applet_no, cmd, argv); } /* re-exec ourselves with the new arguments */ @@ -8754,7 +8808,7 @@ evaltree(union node *n, int flags) status = evaltree(n->nredir.n, flags & EV_TESTED); } if (n->nredir.redirect) - popredir(/*drop:*/ 0, /*restore:*/ 0 /* not sure */); + popredir(/*drop:*/ 0); goto setstatus; case NCMD: evalfn = evalcommand; @@ -9902,7 +9956,7 @@ evalcommand(union node *cmd, int flags) out: if (cmd->ncmd.redirect) - popredir(/*drop:*/ cmd_is_exec, /*restore:*/ cmd_is_exec); + popredir(/*drop:*/ cmd_is_exec); unwindredir(redir_stop); unwindlocalvars(localvar_stop); if (lastarg) { @@ -13727,7 +13781,7 @@ int ash_main(int argc UNUSED_PARAM, char **argv) * Testcase: ash -c 'exec 1>&0' must not complain. */ // if (!sflag) g_parsefile->pf_fd = -1; // ^^ not necessary since now we special-case fd 0 - // in is_hidden_fd() to not be considered "hidden fd" + // in save_fd_on_redirect() evalstring(minusc, sflag ? 0 : EV_EXIT); } diff --git a/shell/ash_test/ash-redir/redir_script.tests b/shell/ash_test/ash-redir/redir_script.tests index ccc497d7b..740daa461 100755 --- a/shell/ash_test/ash-redir/redir_script.tests +++ b/shell/ash_test/ash-redir/redir_script.tests @@ -20,10 +20,15 @@ eval "find_fds $fds" # Shell should not lose that fd. Did it? find_fds -test x"$fds1" = x"$fds" && { echo "Ok: script fd is not closed"; exit 0; } +test x"$fds1" = x"$fds" \ +&& { echo "Ok: script fd is not closed"; exit 0; } + +# One legit way to handle it is to move script fd. For example, if we see that fd 10 moved to fd 11: +test x"$fds1" = x" 10>&- 3>&-" && \ +test x"$fds" = x" 11>&- 3>&-" \ +&& { echo "Ok: script fd is not closed"; exit 0; } echo "Bug: script fd is closed" echo "fds1:$fds1" echo "fds2:$fds" exit 1 - diff --git a/shell/ash_test/ash-redir/redir_to_bad_fd255.right b/shell/ash_test/ash-redir/redir_to_bad_fd255.right new file mode 100644 index 000000000..9c5e35b36 --- /dev/null +++ b/shell/ash_test/ash-redir/redir_to_bad_fd255.right @@ -0,0 +1,2 @@ +./redir_to_bad_fd255.tests: line 2: 255: Bad file descriptor +OK diff --git a/shell/ash_test/ash-redir/redir_to_bad_fd255.tests b/shell/ash_test/ash-redir/redir_to_bad_fd255.tests new file mode 100755 index 000000000..2266af6da --- /dev/null +++ b/shell/ash_test/ash-redir/redir_to_bad_fd255.tests @@ -0,0 +1,3 @@ +# ash uses fd 10 (usually) for reading the script +echo LOST >&255 +echo OK diff --git a/shell/ash_test/ash-redir/redir_to_bad_fd3.right b/shell/ash_test/ash-redir/redir_to_bad_fd3.right new file mode 100644 index 000000000..895a4a0a6 --- /dev/null +++ b/shell/ash_test/ash-redir/redir_to_bad_fd3.right @@ -0,0 +1,2 @@ +./redir_to_bad_fd3.tests: line 2: 3: Bad file descriptor +OK diff --git a/shell/ash_test/ash-redir/redir_to_bad_fd3.tests b/shell/ash_test/ash-redir/redir_to_bad_fd3.tests new file mode 100755 index 000000000..98c54cfc6 --- /dev/null +++ b/shell/ash_test/ash-redir/redir_to_bad_fd3.tests @@ -0,0 +1,3 @@ +# ash uses fd 10 (usually) for reading the script +echo LOST >&3 +echo OK diff --git a/shell/hush_test/hush-redir/redir_script.tests b/shell/hush_test/hush-redir/redir_script.tests index ccc497d7b..740daa461 100755 --- a/shell/hush_test/hush-redir/redir_script.tests +++ b/shell/hush_test/hush-redir/redir_script.tests @@ -20,10 +20,15 @@ eval "find_fds $fds" # Shell should not lose that fd. Did it? find_fds -test x"$fds1" = x"$fds" && { echo "Ok: script fd is not closed"; exit 0; } +test x"$fds1" = x"$fds" \ +&& { echo "Ok: script fd is not closed"; exit 0; } + +# One legit way to handle it is to move script fd. For example, if we see that fd 10 moved to fd 11: +test x"$fds1" = x" 10>&- 3>&-" && \ +test x"$fds" = x" 11>&- 3>&-" \ +&& { echo "Ok: script fd is not closed"; exit 0; } echo "Bug: script fd is closed" echo "fds1:$fds1" echo "fds2:$fds" exit 1 - diff --git a/shell/hush_test/hush-redir/redir_to_bad_fd255.right b/shell/hush_test/hush-redir/redir_to_bad_fd255.right new file mode 100644 index 000000000..936911ce5 --- /dev/null +++ b/shell/hush_test/hush-redir/redir_to_bad_fd255.right @@ -0,0 +1 @@ +hush: can't duplicate file descriptor: Bad file descriptor diff --git a/shell/hush_test/hush-redir/redir_to_bad_fd255.tests b/shell/hush_test/hush-redir/redir_to_bad_fd255.tests new file mode 100755 index 000000000..2266af6da --- /dev/null +++ b/shell/hush_test/hush-redir/redir_to_bad_fd255.tests @@ -0,0 +1,3 @@ +# ash uses fd 10 (usually) for reading the script +echo LOST >&255 +echo OK diff --git a/shell/hush_test/hush-redir/redir_to_bad_fd3.right b/shell/hush_test/hush-redir/redir_to_bad_fd3.right new file mode 100644 index 000000000..936911ce5 --- /dev/null +++ b/shell/hush_test/hush-redir/redir_to_bad_fd3.right @@ -0,0 +1 @@ +hush: can't duplicate file descriptor: Bad file descriptor diff --git a/shell/hush_test/hush-redir/redir_to_bad_fd3.tests b/shell/hush_test/hush-redir/redir_to_bad_fd3.tests new file mode 100755 index 000000000..98c54cfc6 --- /dev/null +++ b/shell/hush_test/hush-redir/redir_to_bad_fd3.tests @@ -0,0 +1,3 @@ +# ash uses fd 10 (usually) for reading the script +echo LOST >&3 +echo OK -- cgit v1.2.3-55-g6feb From 32fdf2f9fc9a617918672d71579f4ad42eb9bde9 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 31 Jul 2017 04:32:06 +0200 Subject: ash,hush: ">&10" redirects to script/tty fds should not work The fact that shell has open fds to tty and/or scripts should be unobservable, if possible. In particular, if redirect tries to dup one of them via ">&script_fd", it's better to pretend that script_fd is closed, and thus redirect fails with EBADF. Fixes these two testcase failures: ash-redir/redir_to_bad_fd.tests hush-redir/redir_to_bad_fd3.tests function old new delta redirect 1018 1129 +111 setup_redirects 250 359 +109 readtoken1 2651 2655 +4 cmdloop 185 187 +2 changepath 194 195 +1 save_fd_on_redirect 203 194 -9 evaltree 501 484 -17 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 5/2 up/down: 227/-26) Total: 201 bytes text data bss dec hex filename 914553 485 6848 921886 e111e busybox_old 914754 485 6848 922087 e11e7 busybox_unstripped Signed-off-by: Denys Vlasenko --- shell/ash.c | 39 ++++++++++++++++++++++++++++++--------- shell/hush.c | 52 ++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 72 insertions(+), 19 deletions(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index ac676c2dc..5c2e06599 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -5503,6 +5503,31 @@ save_fd_on_redirect(int fd, int avoid_fd, struct redirtab *sq) return 0; /* "we did not close fd" */ } +static int +internally_opened_fd(int fd, struct redirtab *sq) +{ + int i; +#if JOBS + if (fd == ttyfd) + return 1; +#endif + /* If this one of script's fds? */ + if (fd != 0) { + struct parsefile *pf = g_parsefile; + while (pf) { + if (fd == pf->pf_fd) + return 1; + pf = pf->prev; + } + } + + if (sq) for (i = 0; i < sq->pair_count && sq->two_fd[i].orig_fd != EMPTY; i++) { + if (fd == sq->two_fd[i].moved_to) + return 1; + } + return 0; +} + /* * Process a list of redirection commands. If the REDIR_PUSH flag is set, * old file descriptors are stashed away so that the redirection can be @@ -5567,15 +5592,11 @@ redirect(union node *redir, int flags) close(fd); } } else { -///TODO: if _newfd_ is a script fd or saved fd, then simulate EBADF! -//if (newfd == ttyfd) { -// errno = EBADF; -// ash_msg_and_raise_perror("A %d", newfd); -//} -//if (newfd == g_parsefile->pf_fd) { -// errno = EBADF; -// ash_msg_and_raise_perror("B %d", newfd); -//} + /* if newfd is a script fd or saved fd, simulate EBADF */ + if (internally_opened_fd(newfd, sv)) { + errno = EBADF; + ash_msg_and_raise_perror("%d", newfd); + } dup2_or_raise(newfd, fd); if (close_fd >= 0) /* "N>FILE" or ">&FILE" or heredoc? */ close(close_fd); diff --git a/shell/hush.c b/shell/hush.c index d4101d66f..cc785d36b 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -1546,6 +1546,16 @@ static void close_all_FILE_list(void) } } #endif +static int fd_in_FILEs(int fd) +{ + struct FILE_list *fl = G.FILE_list; + while (fl) { + if (fl->fd == fd) + return 1; + fl = fl->next; + } + return 0; +} /* Helpers for setting new $n and restoring them back @@ -6686,9 +6696,9 @@ static struct squirrel *append_squirrel(struct squirrel *sq, int i, int orig, in static struct squirrel *add_squirrel(struct squirrel *sq, int fd, int avoid_fd) { int moved_to; - int i = 0; + int i; - if (sq) while (sq[i].orig_fd >= 0) { + if (sq) for (i = 0; sq[i].orig_fd >= 0; i++) { /* If we collide with an already moved fd... */ if (fd == sq[i].moved_to) { sq[i].moved_to = fcntl_F_DUPFD(sq[i].moved_to, avoid_fd); @@ -6702,7 +6712,6 @@ static struct squirrel *add_squirrel(struct squirrel *sq, int fd, int avoid_fd) debug_printf_redir("redirect_fd %d: already moved\n", fd); return sq; } - i++; } /* If this fd is open, we move and remember it; if it's closed, moved_to = -1 */ @@ -6717,8 +6726,7 @@ static struct squirrel *add_squirrel_closed(struct squirrel *sq, int fd) { int i; - i = 0; - if (sq) while (sq[i].orig_fd >= 0) { + if (sq) for (i = 0; sq[i].orig_fd >= 0; i++) { /* If we collide with an already moved fd... */ if (fd == sq[i].orig_fd) { /* Examples: @@ -6730,7 +6738,6 @@ static struct squirrel *add_squirrel_closed(struct squirrel *sq, int fd) debug_printf_redir("redirect_fd %d: already moved or closed\n", fd); return sq; } - i++; } debug_printf_redir("redirect_fd %d: previous fd was closed\n", fd); @@ -6747,7 +6754,8 @@ static int save_fd_on_redirect(int fd, int avoid_fd, struct squirrel **sqp) avoid_fd = 9; #if ENABLE_HUSH_INTERACTIVE - if (fd != 0 && fd == G.interactive_fd) { + if (fd == G.interactive_fd) { + /* Testcase: "ls -l /proc/$$/fd 255>&-" should work */ G.interactive_fd = xdup_CLOEXEC_and_close(G.interactive_fd, avoid_fd); debug_printf_redir("redirect_fd %d: matches interactive_fd, moving it to %d\n", fd, G.interactive_fd); return 1; /* "we closed fd" */ @@ -6775,8 +6783,8 @@ static int save_fd_on_redirect(int fd, int avoid_fd, struct squirrel **sqp) static void restore_redirects(struct squirrel *sq) { if (sq) { - int i = 0; - while (sq[i].orig_fd >= 0) { + int i; + for (i = 0; sq[i].orig_fd >= 0; i++) { if (sq[i].moved_to >= 0) { /* We simply die on error */ debug_printf_redir("restoring redirected fd from %d to %d\n", sq[i].moved_to, sq[i].orig_fd); @@ -6786,7 +6794,6 @@ static void restore_redirects(struct squirrel *sq) debug_printf_redir("restoring redirected fd %d: closing it\n", sq[i].orig_fd); close(sq[i].orig_fd); } - i++; } free(sq); } @@ -6796,6 +6803,25 @@ static void restore_redirects(struct squirrel *sq) restore_redirected_FILEs(); } +static int internally_opened_fd(int fd, struct squirrel *sq) +{ + int i; + +#if ENABLE_HUSH_INTERACTIVE + if (fd == G.interactive_fd) + return 1; +#endif + /* If this one of script's fds? */ + if (fd_in_FILEs(fd)) + return 1; + + if (sq) for (i = 0; sq[i].orig_fd >= 0; i++) { + if (fd == sq[i].moved_to) + return 1; + } + return 0; +} + /* squirrel != NULL means we squirrel away copies of stdin, stdout, * and stderr if they are redirected. */ static int setup_redirects(struct command *prog, struct squirrel **sqp) @@ -6878,6 +6904,12 @@ static int setup_redirects(struct command *prog, struct squirrel **sqp) * and second redirect closes 3! Restore code then closes 3 again. */ } else { + /* if newfd is a script fd or saved fd, simulate EBADF */ + if (internally_opened_fd(newfd, sqp ? *sqp : NULL)) { + //errno = EBADF; + //bb_perror_msg_and_die("can't duplicate file descriptor"); + newfd = -1; /* same effect as code above */ + } xdup2(newfd, redir->rd_fd); if (redir->rd_dup == REDIRFD_TO_FILE) /* "rd_fd > FILE" */ -- cgit v1.2.3-55-g6feb From bf1c344dfdc6f38ad6aa81c10b7b050e0dfc5d96 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 31 Jul 2017 04:54:53 +0200 Subject: hush: if STANDALONE, close interactive fd for NOEXECed children function old new delta pseudo_exec_argv 291 305 +14 Signed-off-by: Denys Vlasenko --- shell/hush.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) (limited to 'shell') diff --git a/shell/hush.c b/shell/hush.c index cc785d36b..8e9e0e9e8 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -6803,6 +6803,15 @@ static void restore_redirects(struct squirrel *sq) restore_redirected_FILEs(); } +#if ENABLE_FEATURE_SH_STANDALONE && BB_MMU +static void close_saved_fds_and_FILE_list(void) +{ + if (G_interactive_fd) + close(G_interactive_fd); + close_all_FILE_list(); +} +#endif + static int internally_opened_fd(int fd, struct squirrel *sq) { int i; @@ -7325,8 +7334,12 @@ static NOINLINE void pseudo_exec_argv(nommu_save_t *nommu_save, if (a >= 0) { # if BB_MMU /* see above why on NOMMU it is not allowed */ if (APPLET_IS_NOEXEC(a)) { - /* Do not leak open fds from opened script files etc */ - close_all_FILE_list(); + /* Do not leak open fds from opened script files etc. + * Testcase: interactive "ls -l /proc/self/fd" + * should not show tty fd open. + */ + close_saved_fds_and_FILE_list(); +///FIXME: should also close saved redir fds debug_printf_exec("running applet '%s'\n", argv[0]); run_applet_no_and_exit(a, argv[0], argv); } -- cgit v1.2.3-55-g6feb From 75481d363454a3e0640a5bd0f78d2cc4403e6235 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 31 Jul 2017 05:27:09 +0200 Subject: hush: functions have priority over builtins (!) function old new delta pseudo_exec_argv 291 305 +14 run_pipe 1560 1555 -5 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 1/1 up/down: 14/-5) Total: 9 bytes Signed-off-by: Denys Vlasenko --- .../ash-misc/func_prio_over_builtins.right | 5 ++++ .../ash-misc/func_prio_over_builtins.tests | 5 ++++ shell/hush.c | 33 +++++++++++++--------- .../hush-misc/func_prio_over_builtins.right | 5 ++++ .../hush-misc/func_prio_over_builtins.tests | 5 ++++ 5 files changed, 39 insertions(+), 14 deletions(-) create mode 100644 shell/ash_test/ash-misc/func_prio_over_builtins.right create mode 100755 shell/ash_test/ash-misc/func_prio_over_builtins.tests create mode 100644 shell/hush_test/hush-misc/func_prio_over_builtins.right create mode 100755 shell/hush_test/hush-misc/func_prio_over_builtins.tests (limited to 'shell') diff --git a/shell/ash_test/ash-misc/func_prio_over_builtins.right b/shell/ash_test/ash-misc/func_prio_over_builtins.right new file mode 100644 index 000000000..54e56dff4 --- /dev/null +++ b/shell/ash_test/ash-misc/func_prio_over_builtins.right @@ -0,0 +1,5 @@ +YES +YES +YES +YES +Ok:YES diff --git a/shell/ash_test/ash-misc/func_prio_over_builtins.tests b/shell/ash_test/ash-misc/func_prio_over_builtins.tests new file mode 100755 index 000000000..4f71bfda0 --- /dev/null +++ b/shell/ash_test/ash-misc/func_prio_over_builtins.tests @@ -0,0 +1,5 @@ +true() { echo YES >&2; } +true +true | true +(true) +echo Ok:`true 2>&1` diff --git a/shell/hush.c b/shell/hush.c index 8e9e0e9e8..2fba637ae 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -7090,11 +7090,13 @@ static void exec_function(char ***to_free, argv[0] = G.global_argv[0]; G.global_argv = argv; G.global_argc = n = 1 + string_array_len(argv + 1); +//? close_saved_fds_and_FILE_list(); /* On MMU, funcp->body is always non-NULL */ n = run_list(funcp->body); fflush_all(); _exit(n); # else +//? close_saved_fds_and_FILE_list(); re_execute_shell(to_free, funcp->body_as_string, G.global_argv[0], @@ -7180,6 +7182,7 @@ static void exec_builtin(char ***to_free, #if BB_MMU int rcode; fflush_all(); +//? close_saved_fds_and_FILE_list(); rcode = x->b_function(argv); fflush_all(); _exit(rcode); @@ -7301,6 +7304,16 @@ static NOINLINE void pseudo_exec_argv(nommu_save_t *nommu_save, goto skip; #endif +#if ENABLE_HUSH_FUNCTIONS + /* Check if the command matches any functions (this goes before bltins) */ + { + const struct function *funcp = find_function(argv[0]); + if (funcp) { + exec_function(&nommu_save->argv_from_re_execing, funcp, argv); + } + } +#endif + /* Check if the command matches any of the builtins. * Depending on context, this might be redundant. But it's * easier to waste a few CPU cycles than it is to figure out @@ -7317,15 +7330,6 @@ static NOINLINE void pseudo_exec_argv(nommu_save_t *nommu_save, exec_builtin(&nommu_save->argv_from_re_execing, x, argv); } } -#if ENABLE_HUSH_FUNCTIONS - /* Check if the command matches any functions */ - { - const struct function *funcp = find_function(argv[0]); - if (funcp) { - exec_function(&nommu_save->argv_from_re_execing, funcp, argv); - } - } -#endif #if ENABLE_FEATURE_SH_STANDALONE /* Check if the command matches any busybox applets */ @@ -7339,7 +7343,7 @@ static NOINLINE void pseudo_exec_argv(nommu_save_t *nommu_save, * should not show tty fd open. */ close_saved_fds_and_FILE_list(); -///FIXME: should also close saved redir fds +//FIXME: should also close saved redir fds debug_printf_exec("running applet '%s'\n", argv[0]); run_applet_no_and_exit(a, argv[0], argv); } @@ -7976,12 +7980,13 @@ static NOINLINE int run_pipe(struct pipe *pi) return G.last_exitcode; } - x = find_builtin(argv_expanded[0]); #if ENABLE_HUSH_FUNCTIONS - funcp = NULL; - if (!x) - funcp = find_function(argv_expanded[0]); + /* Check if argv[0] matches any functions (this goes before bltins) */ + funcp = find_function(argv_expanded[0]); #endif + x = NULL; + if (!funcp) + x = find_builtin(argv_expanded[0]); if (x || funcp) { if (!funcp) { if (x->b_function == builtin_exec && argv_expanded[1] == NULL) { diff --git a/shell/hush_test/hush-misc/func_prio_over_builtins.right b/shell/hush_test/hush-misc/func_prio_over_builtins.right new file mode 100644 index 000000000..54e56dff4 --- /dev/null +++ b/shell/hush_test/hush-misc/func_prio_over_builtins.right @@ -0,0 +1,5 @@ +YES +YES +YES +YES +Ok:YES diff --git a/shell/hush_test/hush-misc/func_prio_over_builtins.tests b/shell/hush_test/hush-misc/func_prio_over_builtins.tests new file mode 100755 index 000000000..4f71bfda0 --- /dev/null +++ b/shell/hush_test/hush-misc/func_prio_over_builtins.tests @@ -0,0 +1,5 @@ +true() { echo YES >&2; } +true +true | true +(true) +echo Ok:`true 2>&1` -- cgit v1.2.3-55-g6feb From d0fff9155bc55d9576be4c6ba5f656f5b4856ac4 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 31 Jul 2017 14:32:18 +0200 Subject: ash: fix display of ">&-" redirect in job strings function old new delta cmdtxt 558 569 +11 Signed-off-by: Denys Vlasenko --- shell/ash.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index 5c2e06599..9c61ce618 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -4642,6 +4642,10 @@ cmdputs(const char *s) /* These can only happen inside quotes */ cc[0] = c; str = cc; +//FIXME: +// $ true $$ & +// $ +// [1]+ Done true ${\$} <<=== BUG: ${\$} is not a valid way to write $$ (${$} would be ok) c = '\\'; break; default: @@ -4823,7 +4827,10 @@ cmdtxt(union node *n) cmdputs(utoa(n->nfile.fd)); cmdputs(p); if (n->type == NTOFD || n->type == NFROMFD) { - cmdputs(utoa(n->ndup.dupfd)); + if (n->ndup.dupfd >= 0) + cmdputs(utoa(n->ndup.dupfd)); + else + cmdputs("-"); break; } n = n->nfile.fname; -- cgit v1.2.3-55-g6feb From 5b3d2eb327ce7e1c55c6c94bc70782f733d59990 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 31 Jul 2017 18:02:28 +0200 Subject: hush: fix "true | func_with_return" not allowing return. function old new delta pseudo_exec_argv 305 312 +7 Signed-off-by: Denys Vlasenko --- shell/ash_test/ash-misc/func6.right | 2 -- shell/ash_test/ash-misc/func6.tests | 11 ------- shell/ash_test/ash-misc/func_return1.right | 2 ++ shell/ash_test/ash-misc/func_return1.tests | 11 +++++++ shell/ash_test/ash-misc/func_return2.right | 2 ++ shell/ash_test/ash-misc/func_return2.tests | 6 ++++ shell/hush.c | 44 +++++++++++++++++++++++----- shell/hush_test/hush-misc/func6.right | 2 -- shell/hush_test/hush-misc/func6.tests | 11 ------- shell/hush_test/hush-misc/func_return1.right | 2 ++ shell/hush_test/hush-misc/func_return1.tests | 11 +++++++ shell/hush_test/hush-misc/func_return2.right | 2 ++ shell/hush_test/hush-misc/func_return2.tests | 6 ++++ 13 files changed, 78 insertions(+), 34 deletions(-) delete mode 100644 shell/ash_test/ash-misc/func6.right delete mode 100755 shell/ash_test/ash-misc/func6.tests create mode 100644 shell/ash_test/ash-misc/func_return1.right create mode 100755 shell/ash_test/ash-misc/func_return1.tests create mode 100644 shell/ash_test/ash-misc/func_return2.right create mode 100755 shell/ash_test/ash-misc/func_return2.tests delete mode 100644 shell/hush_test/hush-misc/func6.right delete mode 100755 shell/hush_test/hush-misc/func6.tests create mode 100644 shell/hush_test/hush-misc/func_return1.right create mode 100755 shell/hush_test/hush-misc/func_return1.tests create mode 100644 shell/hush_test/hush-misc/func_return2.right create mode 100755 shell/hush_test/hush-misc/func_return2.tests (limited to 'shell') diff --git a/shell/ash_test/ash-misc/func6.right b/shell/ash_test/ash-misc/func6.right deleted file mode 100644 index 0ebd8e5a3..000000000 --- a/shell/ash_test/ash-misc/func6.right +++ /dev/null @@ -1,2 +0,0 @@ -Two:2 -Two:2 diff --git a/shell/ash_test/ash-misc/func6.tests b/shell/ash_test/ash-misc/func6.tests deleted file mode 100755 index 029c3e85e..000000000 --- a/shell/ash_test/ash-misc/func6.tests +++ /dev/null @@ -1,11 +0,0 @@ -f1() { - while return 2; do :; done -} -f1 -echo Two:$? - -f2() { - while :; do return 2; done -} -f2 -echo Two:$? diff --git a/shell/ash_test/ash-misc/func_return1.right b/shell/ash_test/ash-misc/func_return1.right new file mode 100644 index 000000000..0ebd8e5a3 --- /dev/null +++ b/shell/ash_test/ash-misc/func_return1.right @@ -0,0 +1,2 @@ +Two:2 +Two:2 diff --git a/shell/ash_test/ash-misc/func_return1.tests b/shell/ash_test/ash-misc/func_return1.tests new file mode 100755 index 000000000..029c3e85e --- /dev/null +++ b/shell/ash_test/ash-misc/func_return1.tests @@ -0,0 +1,11 @@ +f1() { + while return 2; do :; done +} +f1 +echo Two:$? + +f2() { + while :; do return 2; done +} +f2 +echo Two:$? diff --git a/shell/ash_test/ash-misc/func_return2.right b/shell/ash_test/ash-misc/func_return2.right new file mode 100644 index 000000000..0ebd8e5a3 --- /dev/null +++ b/shell/ash_test/ash-misc/func_return2.right @@ -0,0 +1,2 @@ +Two:2 +Two:2 diff --git a/shell/ash_test/ash-misc/func_return2.tests b/shell/ash_test/ash-misc/func_return2.tests new file mode 100755 index 000000000..a049dd180 --- /dev/null +++ b/shell/ash_test/ash-misc/func_return2.tests @@ -0,0 +1,6 @@ +f1() { return 2; } +f1 +echo Two:$? +false +true | f1 +echo Two:$? diff --git a/shell/hush.c b/shell/hush.c index 2fba637ae..5c8e00743 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -6804,7 +6804,7 @@ static void restore_redirects(struct squirrel *sq) } #if ENABLE_FEATURE_SH_STANDALONE && BB_MMU -static void close_saved_fds_and_FILE_list(void) +static void close_saved_fds_and_FILE_fds(void) { if (G_interactive_fd) close(G_interactive_fd); @@ -7090,13 +7090,35 @@ static void exec_function(char ***to_free, argv[0] = G.global_argv[0]; G.global_argv = argv; G.global_argc = n = 1 + string_array_len(argv + 1); -//? close_saved_fds_and_FILE_list(); + +// Example when we are here: "cmd | func" +// func will run with saved-redirect fds open. +// $ f() { echo /proc/self/fd/*; } +// $ true | f +// /proc/self/fd/0 /proc/self/fd/1 /proc/self/fd/2 /proc/self/fd/255 /proc/self/fd/3 +// stdio^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ G_interactive_fd^ DIR fd for glob +// Same in script: +// $ . ./SCRIPT +// /proc/self/fd/0 /proc/self/fd/1 /proc/self/fd/2 /proc/self/fd/255 /proc/self/fd/3 /proc/self/fd/4 +// stdio^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ G_interactive_fd^ opened ./SCRIPT DIR fd for glob +// They are CLOEXEC so external programs won't see them, but +// for "more correctness" we might want to close those extra fds here: +//? close_saved_fds_and_FILE_fds(); + + /* "we are in function, ok to use return" */ + G_flag_return_in_progress = -1; + IF_HUSH_LOCAL(G.func_nest_level++;) + + G_flag_return_in_progress = -1; /* On MMU, funcp->body is always non-NULL */ n = run_list(funcp->body); fflush_all(); _exit(n); # else -//? close_saved_fds_and_FILE_list(); +//? close_saved_fds_and_FILE_fds(); + +//TODO: check whether "true | func_with_return" works + re_execute_shell(to_free, funcp->body_as_string, G.global_argv[0], @@ -7116,9 +7138,7 @@ static int run_function(const struct function *funcp, char **argv) /* "we are in function, ok to use return" */ sv_flg = G_flag_return_in_progress; G_flag_return_in_progress = -1; -# if ENABLE_HUSH_LOCAL - G.func_nest_level++; -# endif + IF_HUSH_LOCAL(G.func_nest_level++;) /* On MMU, funcp->body is always non-NULL */ # if !BB_MMU @@ -7182,7 +7202,7 @@ static void exec_builtin(char ***to_free, #if BB_MMU int rcode; fflush_all(); -//? close_saved_fds_and_FILE_list(); +//? close_saved_fds_and_FILE_fds(); rcode = x->b_function(argv); fflush_all(); _exit(rcode); @@ -7342,7 +7362,7 @@ static NOINLINE void pseudo_exec_argv(nommu_save_t *nommu_save, * Testcase: interactive "ls -l /proc/self/fd" * should not show tty fd open. */ - close_saved_fds_and_FILE_list(); + close_saved_fds_and_FILE_fds(); //FIXME: should also close saved redir fds debug_printf_exec("running applet '%s'\n", argv[0]); run_applet_no_and_exit(a, argv[0], argv); @@ -9253,6 +9273,14 @@ static int FAST_FUNC builtin_exec(char **argv) if (G_saved_tty_pgrp && getpid() == G.root_pid) tcsetpgrp(G_interactive_fd, G_saved_tty_pgrp); + /* Saved-redirect fds, script fds and G_interactive_fd are still + * open here. However, they are all CLOEXEC, and execv below + * closes them. Try interactive "exec ls -l /proc/self/fd", + * it should show no extra open fds in the "ls" process. + * If we'd try to run builtins/NOEXECs, this would need improving. + */ + //close_saved_fds_and_FILE_fds(); + /* TODO: if exec fails, bash does NOT exit! We do. * We'll need to undo trap cleanup (it's inside execvp_or_die) * and tcsetpgrp, and this is inherently racy. diff --git a/shell/hush_test/hush-misc/func6.right b/shell/hush_test/hush-misc/func6.right deleted file mode 100644 index 0ebd8e5a3..000000000 --- a/shell/hush_test/hush-misc/func6.right +++ /dev/null @@ -1,2 +0,0 @@ -Two:2 -Two:2 diff --git a/shell/hush_test/hush-misc/func6.tests b/shell/hush_test/hush-misc/func6.tests deleted file mode 100755 index 029c3e85e..000000000 --- a/shell/hush_test/hush-misc/func6.tests +++ /dev/null @@ -1,11 +0,0 @@ -f1() { - while return 2; do :; done -} -f1 -echo Two:$? - -f2() { - while :; do return 2; done -} -f2 -echo Two:$? diff --git a/shell/hush_test/hush-misc/func_return1.right b/shell/hush_test/hush-misc/func_return1.right new file mode 100644 index 000000000..0ebd8e5a3 --- /dev/null +++ b/shell/hush_test/hush-misc/func_return1.right @@ -0,0 +1,2 @@ +Two:2 +Two:2 diff --git a/shell/hush_test/hush-misc/func_return1.tests b/shell/hush_test/hush-misc/func_return1.tests new file mode 100755 index 000000000..029c3e85e --- /dev/null +++ b/shell/hush_test/hush-misc/func_return1.tests @@ -0,0 +1,11 @@ +f1() { + while return 2; do :; done +} +f1 +echo Two:$? + +f2() { + while :; do return 2; done +} +f2 +echo Two:$? diff --git a/shell/hush_test/hush-misc/func_return2.right b/shell/hush_test/hush-misc/func_return2.right new file mode 100644 index 000000000..0ebd8e5a3 --- /dev/null +++ b/shell/hush_test/hush-misc/func_return2.right @@ -0,0 +1,2 @@ +Two:2 +Two:2 diff --git a/shell/hush_test/hush-misc/func_return2.tests b/shell/hush_test/hush-misc/func_return2.tests new file mode 100755 index 000000000..a049dd180 --- /dev/null +++ b/shell/hush_test/hush-misc/func_return2.tests @@ -0,0 +1,6 @@ +f1() { return 2; } +f1 +echo Two:$? +false +true | f1 +echo Two:$? -- cgit v1.2.3-55-g6feb From cee603d921594fc779bc26a200dfb010cd62ab92 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 31 Jul 2017 18:06:07 +0200 Subject: hush: remove redundant "G_flag_return_in_progress = -1" Signed-off-by: Denys Vlasenko --- shell/hush.c | 1 - 1 file changed, 1 deletion(-) (limited to 'shell') diff --git a/shell/hush.c b/shell/hush.c index 5c8e00743..9f946d82f 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -7109,7 +7109,6 @@ static void exec_function(char ***to_free, G_flag_return_in_progress = -1; IF_HUSH_LOCAL(G.func_nest_level++;) - G_flag_return_in_progress = -1; /* On MMU, funcp->body is always non-NULL */ n = run_list(funcp->body); fflush_all(); -- cgit v1.2.3-55-g6feb From ec05df13b0f3bc69074909f078f981f417d95c89 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 31 Jul 2017 19:43:47 +0200 Subject: ash: align --login code with dash Upstream commit: Date: Sun, 13 Jul 2008 22:34:50 +0800 [OPTIONS] Added support for -l This patch adds support for the -l option (login shell) as required by the LSB. Signed-off-by: Herbert Xu It's a bit bigger, but gets rid of one global variable function old new delta options 554 576 +22 Signed-off-by: Denys Vlasenko --- shell/ash.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index 9c61ce618..1deae7c2f 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -328,7 +328,6 @@ struct globals_misc { #define EXERROR 1 /* a generic error */ #define EXEXIT 4 /* exit the shell */ - smallint isloginsh; char nullstr[1]; /* zero length string */ char optlist[NOPTS]; @@ -397,7 +396,6 @@ extern struct globals_misc *const ash_ptr_to_globals_misc; #define pending_int (G_misc.pending_int ) #define got_sigchld (G_misc.got_sigchld ) #define pending_sig (G_misc.pending_sig ) -#define isloginsh (G_misc.isloginsh ) #define nullstr (G_misc.nullstr ) #define optlist (G_misc.optlist ) #define sigmode (G_misc.sigmode ) @@ -10721,7 +10719,7 @@ setoption(int flag, int val) /* NOTREACHED */ } static int -options(int cmdline) +options(int cmdline, int *login_sh) { char *p; int val; @@ -10762,11 +10760,14 @@ options(int cmdline) if (*argptr) argptr++; } else if (cmdline && (c == 'l')) { /* -l or +l == --login */ - isloginsh = 1; + if (login_sh) + *login_sh = 1; /* bash does not accept +-login, we also won't */ } else if (cmdline && val && (c == '-')) { /* long options */ - if (strcmp(p, "login") == 0) - isloginsh = 1; + if (strcmp(p, "login") == 0) { + if (login_sh) + *login_sh = 1; + } break; } else { setoption(c, val); @@ -10850,7 +10851,7 @@ setcmd(int argc UNUSED_PARAM, char **argv UNUSED_PARAM) return showvars(nullstr, 0, VUNSET); INT_OFF; - retval = options(/*cmdline:*/ 0); + retval = options(/*cmdline:*/ 0, NULL); if (retval == 0) { /* if no parse error... */ optschanged(); if (*argptr != NULL) { @@ -13602,21 +13603,23 @@ init(void) /* * Process the shell command line arguments. */ -static void +static int procargs(char **argv) { int i; const char *xminusc; char **xargv; + int login_sh; xargv = argv; + login_sh = xargv[0] && xargv[0][0] == '-'; arg0 = xargv[0]; /* if (xargv[0]) - mmm, this is always true! */ xargv++; for (i = 0; i < NOPTS; i++) optlist[i] = 2; argptr = xargv; - if (options(/*cmdline:*/ 1)) { + if (options(/*cmdline:*/ 1, &login_sh)) { /* it already printed err message */ raise_exception(EXERROR); } @@ -13660,6 +13663,8 @@ procargs(char **argv) xargv++; } optschanged(); + + return login_sh; } /* @@ -13720,6 +13725,7 @@ int ash_main(int argc UNUSED_PARAM, char **argv) volatile smallint state; struct jmploc jmploc; struct stackmark smark; + int login_sh; /* Initialize global data */ INIT_G_misc(); @@ -13768,15 +13774,13 @@ int ash_main(int argc UNUSED_PARAM, char **argv) init(); setstackmark(&smark); - procargs(argv); + login_sh = procargs(argv); #if DEBUG TRACE(("Shell args: ")); trace_puts_args(argv); #endif - if (argv[0] && argv[0][0] == '-') - isloginsh = 1; - if (isloginsh) { + if (login_sh) { const char *hp; state = 1; -- cgit v1.2.3-55-g6feb