From 5b3d2eb327ce7e1c55c6c94bc70782f733d59990 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 31 Jul 2017 18:02:28 +0200 Subject: [PATCH] hush: fix "true | func_with_return" not allowing return. function old new delta pseudo_exec_argv 305 312 +7 Signed-off-by: Denys Vlasenko --- .../{func6.right => func_return1.right} | 0 .../{func6.tests => func_return1.tests} | 0 .../ash-misc/func_return2.right} | 0 shell/ash_test/ash-misc/func_return2.tests | 6 +++ shell/hush.c | 44 +++++++++++++++---- shell/hush_test/hush-misc/func_return1.right | 2 + .../{func6.tests => func_return1.tests} | 0 shell/hush_test/hush-misc/func_return2.right | 2 + shell/hush_test/hush-misc/func_return2.tests | 6 +++ 9 files changed, 52 insertions(+), 8 deletions(-) rename shell/ash_test/ash-misc/{func6.right => func_return1.right} (100%) rename shell/ash_test/ash-misc/{func6.tests => func_return1.tests} (100%) rename shell/{hush_test/hush-misc/func6.right => ash_test/ash-misc/func_return2.right} (100%) create mode 100755 shell/ash_test/ash-misc/func_return2.tests create mode 100644 shell/hush_test/hush-misc/func_return1.right rename shell/hush_test/hush-misc/{func6.tests => func_return1.tests} (100%) create mode 100644 shell/hush_test/hush-misc/func_return2.right create mode 100755 shell/hush_test/hush-misc/func_return2.tests diff --git a/shell/ash_test/ash-misc/func6.right b/shell/ash_test/ash-misc/func_return1.right similarity index 100% rename from shell/ash_test/ash-misc/func6.right rename to shell/ash_test/ash-misc/func_return1.right diff --git a/shell/ash_test/ash-misc/func6.tests b/shell/ash_test/ash-misc/func_return1.tests similarity index 100% rename from shell/ash_test/ash-misc/func6.tests rename to shell/ash_test/ash-misc/func_return1.tests diff --git a/shell/hush_test/hush-misc/func6.right b/shell/ash_test/ash-misc/func_return2.right similarity index 100% rename from shell/hush_test/hush-misc/func6.right rename to shell/ash_test/ash-misc/func_return2.right 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/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/func6.tests b/shell/hush_test/hush-misc/func_return1.tests similarity index 100% rename from shell/hush_test/hush-misc/func6.tests rename to shell/hush_test/hush-misc/func_return1.tests 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:$? -- 2.25.1