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/hush.c | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) (limited to 'shell/hush.c') 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) { -- 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/hush.c') 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 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/hush.c') 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/hush.c') 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/hush.c') 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 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/hush.c') 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/hush.c') 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