NOFORK fixes

"rm -i FILE" and "yes" can now be interrupted by ^C in hush.
This also now works:

$ usleep 19999999
^C
$ echo $?
130

function                                             old     new   delta
run_pipe                                            1668    1711     +43
pseudo_exec_argv                                     312     321      +9
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 2/0 up/down: 52/0)               Total: 52 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

diff --git a/coreutils/rm.c b/coreutils/rm.c
index cec34cb9de70229bcccc8b1625dc020e54e34e33..158c69be5e6d2cd58ee984e375b3641a428a2122 100644 (file)
--- a/coreutils/rm.c
+++ b/coreutils/rm.c
@@ -16,7 +16,8 @@
 //config:      help
 //config:        rm is used to remove files or directories.
 
-//applet:IF_RM(APPLET_NOFORK(rm, rm, BB_DIR_BIN, BB_SUID_DROP, rm))
+//applet:IF_RM(APPLET_NOEXEC(rm, rm, BB_DIR_BIN, BB_SUID_DROP, rm))
+/* was NOFORK, but then "rm -i FILE" can't be ^C'ed if run by hush */
 
 //kbuild:lib-$(CONFIG_RM) += rm.o
 
@@ -36,7 +37,7 @@
 
 #include "libbb.h"
 
-/* This is a NOFORK applet. Be very careful! */
+/* This is a NOEXEC applet. Be very careful! */
 
 int rm_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int rm_main(int argc UNUSED_PARAM, char **argv)

diff --git a/coreutils/seq.c b/coreutils/seq.c
index ed4946b05f07b1de5070ebd3b84680bf3792bb46..d5c6451cd60757f4ea48f05295205e831e9d980c 100644 (file)
--- a/coreutils/seq.c
+++ b/coreutils/seq.c
@@ -12,7 +12,8 @@
 //config:      help
 //config:        print a sequence of numbers
 
-//applet:IF_SEQ(APPLET_NOFORK(seq, seq, BB_DIR_USR_BIN, BB_SUID_DROP, seq))
+//applet:IF_SEQ(APPLET_NOEXEC(seq, seq, BB_DIR_USR_BIN, BB_SUID_DROP, seq))
+/* was NOFORK, but then "seq 1 999999999" can't be ^C'ed if run by hush */
 
 //kbuild:lib-$(CONFIG_SEQ) += seq.o
 
@@ -26,7 +27,7 @@
 
 #include "libbb.h"
 
-/* This is a NOFORK applet. Be very careful! */
+/* This is a NOEXEC applet. Be very careful! */
 
 int seq_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int seq_main(int argc, char **argv)

diff --git a/coreutils/usleep.c b/coreutils/usleep.c
index 7bc30c2a2951bbb92f4625d4b6fef88fc1c60e4b..e68bb4c4dbeb636f292c2133d596af500e930b7c 100644 (file)
--- a/coreutils/usleep.c
+++ b/coreutils/usleep.c
@@ -38,6 +38,13 @@ int usleep_main(int argc UNUSED_PARAM, char **argv)
                bb_show_usage();
        }
 
+       /* Safe wrt NOFORK? (noforks are not allowed to run for
+        * a long time). Try "usleep 99999999" + ^C + "echo $?"
+        * in hush with FEATURE_SH_NOFORK=y.
+        * At least on uclibc, usleep() thanslates to nanosleep()
+        * which returns early on any signal (even caught one),
+        * and uclibc does not loop back on EINTR.
+        */
        usleep(xatou(argv[1]));
 
        return EXIT_SUCCESS;

diff --git a/coreutils/yes.c b/coreutils/yes.c
index 81d875589cc98f011ae494c84593efa2d0573897..6b863c3c9a95e04fd383f35d43093206d969aa61 100644 (file)
--- a/coreutils/yes.c
+++ b/coreutils/yes.c
@@ -17,7 +17,8 @@
 //config:        yes is used to repeatedly output a specific string, or
 //config:        the default string `y'.
 
-//applet:IF_YES(APPLET_NOFORK(yes, yes, BB_DIR_USR_BIN, BB_SUID_DROP, yes))
+//applet:IF_YES(APPLET_NOEXEC(yes, yes, BB_DIR_USR_BIN, BB_SUID_DROP, yes))
+/* was NOFORK, but then yes can't be ^C'ed if run by hush */
 
 //kbuild:lib-$(CONFIG_YES) += yes.o
 

diff --git a/docs/nofork_noexec.txt b/docs/nofork_noexec.txt
index a24dd9c27382069fbc044173e8d363092fa63a6a..0ad4e6e605bfa31caf31916d9552ab6b179ce9c1 100644 (file)
--- a/docs/nofork_noexec.txt
+++ b/docs/nofork_noexec.txt
@@ -52,6 +52,9 @@ xargs, find, shells do it (grep for "spawn_and_wait" and
 This poses much more serious limitations on what applet can do:
 
 * all NOEXEC limitations apply.
+* do not run for a long time or wait for user input:
+  hush shell only handles signals (like ^C) after you return
+  from APPLET_main().
 * do not ever exit() or exec().
   - xfuncs are okay. They are using special trick to return
     to the caller applet instead of dying when they detect "x" condition.

diff --git a/shell/hush.c b/shell/hush.c
index 7cceab30da5ad82beb935d12ac86e71a03e6bc65..2e435cf9fdbf31aa4ff9fe85563b632ca319623e 100644 (file)
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -7063,6 +7063,8 @@ static NOINLINE void pseudo_exec_argv(nommu_save_t *nommu_save,
                        if (APPLET_IS_NOEXEC(a)) {
                                /* Do not leak open fds from opened script files etc */
                                close_all_FILE_list();
+                               /* Without this, "rm -i FILE" can't be ^C'ed: */
+                               switch_off_special_sigs(G.special_sig_mask);
                                debug_printf_exec("running applet '%s'\n", argv[0]);
                                run_applet_no_and_exit(a, argv[0], argv);
                        }
@@ -7725,6 +7727,24 @@ static NOINLINE int run_pipe(struct pipe *pi)
                        add_vars(old_vars);
 /* clean_up_and_ret0: */
                        restore_redirects(squirrel);
+                       /*
+                        * Try "usleep 99999999" + ^C + "echo $?"
+                        * with FEATURE_SH_NOFORK=y.
+                        */
+                       if (!funcp) {
+                               /* It was builtin or nofork.
+                                * if this would be a real fork/execed program,
+                                * it should have died if a fatal sig was received.
+                                * But OTOH, there was no separate process,
+                                * the sig was sent to _shell_, not to non-existing
+                                * child.
+                                * Let's just handle ^C only, this one is obvious:
+                                * we aren't ok with exitcode 0 when ^C was pressed
+                                * during builtin/nofork.
+                                */
+                               if (sigismember(&G.pending_set, SIGINT))
+                                       rcode = 128 + SIGINT;
+                       }
  clean_up_and_ret1:
                        free(argv_expanded);
                        IF_HAS_KEYWORDS(if (pi->pi_inverted) rcode = !rcode;)
@@ -7740,6 +7760,14 @@ static NOINLINE int run_pipe(struct pipe *pi)
                                if (rcode == 0) {
                                        debug_printf_exec(": run_nofork_applet '%s' '%s'...\n",
                                                argv_expanded[0], argv_expanded[1]);
+                                       /*
+                                        * Note: signals (^C) can't interrupt here.
+                                        * We remember them and they will be acted upon
+                                        * after applet returns.
+                                        * This makes applets which can run for a long time
+                                        * and/or wait for user input ineligible for NOFORK:
+                                        * for example, "yes" or "rm" (rm -i waits for input).
+                                        */
                                        rcode = run_nofork_applet(n, argv_expanded);
                                }
                                goto clean_up_and_ret;
@@ -8160,7 +8188,7 @@ static int run_list(struct pipe *pi)
                        /* Last command's pid goes to $! */
                        G.last_bg_pid = pi->cmds[pi->num_cmds - 1].pid;
                        debug_printf_exec(": cmd&: exitcode EXIT_SUCCESS\n");
-/* Check pi->pi_inverted? "! sleep 1 & echo $?": bash says 1. dash and ash says 0 */
+/* Check pi->pi_inverted? "! sleep 1 & echo $?": bash says 1. dash and ash say 0 */
                        rcode = EXIT_SUCCESS;
                        goto check_traps;
                } else {
@@ -9791,6 +9819,7 @@ static int wait_for_child_or_signal(struct pipe *waitfor_pipe, pid_t waitfor_pid
                /* So, did we get a signal? */
                sig = check_and_run_traps();
                if (sig /*&& sig != SIGCHLD - always true */) {
+                       /* Do this for any (non-ignored) signal, not only for ^C */
                        ret = 128 + sig;
                        break;
                }