new NOFORKs: pwdx,kill[all5],ttysize,realpath,readlink NOEXECs: date,resize

function                                             old     new   delta
run_nofork_applet                                    258     280     +22
readlink_main                                        112     123     +11
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 2/0 up/down: 33/0)               Total: 33 bytes

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

diff --git a/NOFORK_NOEXEC.lst b/NOFORK_NOEXEC.lst
index 02eba46e87bfd4b361fc5396d935f973fc26ded2..7073611a4506c3e13a4109197a47f7d4ba229360 100644 (file)
--- a/NOFORK_NOEXEC.lst
+++ b/NOFORK_NOEXEC.lst
@@ -1,11 +1,10 @@
 Why an applet can't be NOFORK or NOEXEC?
 
 Why can't be NOFORK:
-daemon: runs indefinitely
 interactive: may wait for user input, ^C has to work
 spawner: "tool PROG ARGS" which changes program's environment - must fork
 changes state: e.g. environment, signal handlers
-runner: sometimes may run for long time, and/or works with network:
+runner: sometimes may run for long(ish) time, and/or works with network:
        ^C has to work (cat BIGFILE, chmod -R, ftpget, nc)
 
 "runners" can become eligible after hush is taught ^C to interrupt NOFORKs!
@@ -15,9 +14,12 @@ suid: runs under different uid - must fork+exec
 
 Why shouldn't be NOFORK/NOEXEC:
 complex: no immediately obvious reason why NOFORK wouldn't work,
-       but does some non-obvoius operations (example: fuser, lsof, losetup).
-       for NOFORK, nested xmallocs (typical in complex code) is a problem.
+       but does some non-obvoius operations (example: fuser, lsof, losetup);
+       nested xmallocs (typical in complex code) is a problem for NOFORK
 rare: not used often enough to bother optimizing (example: poweroff)
+longterm: often runs for a long time (many seconds), execing would make
+       memory footprint smaller
+daemon: runs indefinitely
 
 [ - NOFORK
 [[ - NOFORK
@@ -31,7 +33,7 @@ arch - NOFORK
 arp
 arping - runner
 ash - interactive
-awk - noexec, runner
+awk - noexec. runner
 base64 - runner
 basename - NOFORK
 beep
@@ -44,63 +46,63 @@ bunzip2 - runner
 busybox
 bzcat - runner
 bzip2 - runner
-cal
+cal - runner: cal -n9999
 cat - runner
 chat
 chattr - runner
-chgrp - noexec, runner
-chmod - noexec, runner
-chown - noexec, runner
+chgrp - noexec. runner
+chmod - noexec. runner
+chown - noexec. runner
 chpasswd - runner (list of "user:password"s from stdin)
 chpst - spawner
 chroot - spawner
 chrt - spawner
 chvt
-cksum - noexec, runner
+cksum - noexec. runner
 clear - NOFORK
 cmp - runner
 comm - runner
 conspy - interactive
-cp - noexec, runner
+cp - noexec. runner
 cpio - runner
 crond - daemon
 crontab
 cryptpw
 cttyhack - spawner
-cut - noexec, runner
-date
+cut - noexec. runner
+date - noexec. nofork candidate(needs to stop messing up env, free xasprintf result, not use xfuncs after xasprintf)
 dc - runner (eats stdin if no params)
-dd - noexec, runner
+dd - noexec. runner
 deallocvt
 delgroup
 deluser
 depmod
-devmem
-df
+devmem - runner, complex (access to device memory may hang)
+df - complex (nested allocs)
 dhcprelay - daemon
 diff - runner
 dirname - NOFORK
-dmesg
+dmesg - runner
 dnsd - daemon
-dnsdomainname
-dos2unix - noexec, runner
+dnsdomainname - DNS resolution may trigger, need ^C
+dos2unix - noexec. runner
 dpkg - runner
-du
+du - runner
 dumpkmap
 dumpleases
 echo - NOFORK
 ed - interactive
 egrep - runner
 eject
-env - noexec, changes state (env)
+env - noexec. changes state (env)
 envdir - spawner
 envuidgid - spawner
 expand - runner
-expr
+expr - complex (nested allocs)
 factor - runner (eats stdin if no params)
 fakeidentd - daemon
 false - NOFORK
-fatattr
+fatattr - complex (xopen+xioctl can leak fd)
 fbset
 fbsplash - runner, interactive
 fdflush
@@ -108,15 +110,15 @@ fdformat - runner
 fdisk - interactive
 fgconsole
 fgrep - runner
-find - noexec, runner
+find - noexec. runner
 findfs - suid
 flash_eraseall
 flash_lock
 flash_unlock
 flashcp
 flock
-fold - noexec, runner
-free
+fold - noexec. runner
+free - nofork candidate(struct globals, needs to close /proc/meminfo fd)
 freeramdisk
 fsck - interactive
 fsck.minix
@@ -134,12 +136,12 @@ groups - noexec
 gunzip - runner
 gzip - runner
 halt - rare
-hd - noexec, runner
+hd - noexec. runner
 hdparm - complex, rare
-head - noexec, runner
-hexdump - noexec, runner
+head - noexec. runner
+hexdump - noexec. runner
 hostid - NOFORK
-hostname
+hostname - DNS resolution may trigger, need ^C
 httpd - daemon
 hush - interactive
 hwclock
@@ -169,11 +171,11 @@ iproute
 iprule
 iptunnel
 kbd_mode
-kill
-killall
-killall5
+kill - NOFORK
+killall - NOFORK
+killall5 - NOFORK
 klogd - daemon
-last
+last - runner (I've got 1300 lines of output when tried it)
 less - interactive
 link - NOFORK
 linux32 - spawner
@@ -189,7 +191,7 @@ losetup - complex
 lpd - daemon
 lpq - runner
 lpr - runner
-ls - noexec, runner
+ls - noexec. runner
 lsattr
 lsmod
 lsof - complex
@@ -203,7 +205,7 @@ lzopcat - runner
 makedevs
 makemime - runner
 man - spawner, interactive
-md5sum - noexec, runner
+md5sum - noexec. runner
 mdev - daemon
 mesg
 microcom - interactive, complex
@@ -225,11 +227,11 @@ mount - suid
 mountpoint
 mpstat
 mt
-mv
+mv - runner (can be noexec?)
 nameif
 nbd-client
 nc - runner
-netstat
+netstat - runner with -c
 nice - spawner
 nl - runner
 nmeter - runner
@@ -240,40 +242,40 @@ od - runner
 openvt - spawner
 partprobe
 passwd - suid
-paste - noexec, runner
+paste - noexec. runner
 patch
-pgrep
-pidof
+pgrep - nofork candidate(xregcomp, procps_scan - are they ok?)
+pidof - nofork candidate(uses find_pid_by_name, is that ok?)
 ping - suid, runner
 ping6 - suid, runner
 pipe_progress
 pivot_root
-pkill
+pkill - nofork candidate(xregcomp, procps_scan - are they ok?)
 pmap
 popmaildir - runner
 poweroff - rare
-powertop - interactive
+powertop - interactive, longterm
 printenv - NOFORK
 printf - NOFORK
 ps
 pscan
 pstree
 pwd - NOFORK
-pwdx
+pwdx - NOFORK
 raidautorun
 rdate
 rdev
-readlink
+readlink - NOFORK
 readprofile
-realpath
+realpath - NOFORK
 reboot - rare
 reformime - runner
 remove-shell
-renice
+renice - nofork candidate(uses getpwnam, is that ok?)
 reset - spawner (execs "stty")
-resize
+resize - noexec. changes state (signal handlers)
 rev - runner
-rm - noexec, rm -i interactive
+rm - noexec. rm -i interactive
 rmdir - NOFORK
 rmmod
 route
@@ -289,7 +291,7 @@ script
 scriptreplay
 sed - runner
 sendmail - runner
-seq - noexec, runner
+seq - noexec. runner
 setarch - spawner
 setconsole
 setfont
@@ -300,22 +302,22 @@ setserial
 setsid - spawner
 setuidgid
 sh - interactive
-sha1sum - noexec, runner
-sha256sum - noexec, runner
-sha3sum - noexec, runner
-sha512sum - noexec, runner
+sha1sum - noexec. runner
+sha256sum - noexec. runner
+sha3sum - noexec. runner
+sha512sum - noexec. runner
 showkey - interactive
 shred - runner
-shuf - noexec, runner
+shuf - noexec. runner
 slattach
 sleep - runner
 smemcap - runner
 softlimit - spawner
-sort - noexec, runner
+sort - noexec. runner
 split - runner
 ssl_client - network
 start-stop-daemon
-stat
+stat - nofork candidate(needs fewer allocs)
 strings - runner
 stty
 su - suid, spawner
@@ -326,11 +328,11 @@ svc
 svlogd - daemon
 swapoff - rare
 swapon - rare
-switch_root - spawner, rare, change state
+switch_root - spawner, rare, changes state
 sync - NOFORK
 sysctl
 syslogd - daemon
-tac - noexec, runner
+tac - noexec. runner
 tail - runner
 tar - runner
 taskset - spawner
@@ -341,9 +343,9 @@ telnetd - daemon
 test - NOFORK
 tftp - runner
 tftpd - daemon
-time - spawner, change state (signals)
-timeout - spawner, change state (signals)
-top - interactive
+time - spawner, changes state (signals)
+timeout - spawner, changes state (signals)
+top - interactive, longterm
 touch - NOFORK
 tr - runner
 traceroute - suid, runner
@@ -351,7 +353,7 @@ traceroute6 - suid, runner
 true - NOFORK
 truncate - NOFORK
 tty - NOFORK
-ttysize
+ttysize - NOFORK
 tunctl
 tune2fs
 ubiattach
@@ -370,14 +372,14 @@ uname - NOFORK
 uncompress - runner
 unexpand - runner
 uniq - runner
-unix2dos - noexec, runner
+unix2dos - noexec. runner
 unlink - NOFORK
 unlzma - runner
 unlzop - runner
 unxz - runner
 unzip - runner
-uptime
-users
+uptime - nofork candidate(is getutxent ok?)
+users - nofork candidate(is getutxent ok?)
 usleep - NOFORK
 uudecode - runner
 uuencode - runner
@@ -395,10 +397,10 @@ which - NOFORK
 who
 whoami - NOFORK
 whois
-xargs - noexec, spawner
-xxd - noexec, runner
+xargs - noexec. spawner
+xxd - noexec. runner
 xz - runner
 xzcat - runner
-yes - noexec, runner
+yes - noexec. runner
 zcat - runner
 zcip - daemon

diff --git a/console-tools/resize.c b/console-tools/resize.c
index 62928a01e90035474fb2136d2a88b6bdabafaaf7..97866673a21fbcd722d538fb88aa015e5f76478e 100644 (file)
--- a/console-tools/resize.c
+++ b/console-tools/resize.c
@@ -23,7 +23,7 @@
 //config:      E.g.:
 //config:      COLUMNS=80;LINES=44;export COLUMNS LINES;
 
-//applet:IF_RESIZE(APPLET(resize, BB_DIR_USR_BIN, BB_SUID_DROP))
+//applet:IF_RESIZE(APPLET_NOEXEC(resize, resize, BB_DIR_USR_BIN, BB_SUID_DROP, resize))
 
 //kbuild:lib-$(CONFIG_RESIZE) += resize.o
 

diff --git a/coreutils/date.c b/coreutils/date.c
index 2c6e1d4df66be615c8d8601a994d73fadfba0c18..89b281646e240ff2bb73c736c1523d3a38f15573 100644 (file)
--- a/coreutils/date.c
+++ b/coreutils/date.c
@@ -58,7 +58,7 @@
 //config:      the same format. With it on, 'date DATE' additionally supports
 //config:      MMDDhhmm[[YY]YY][.ss] format.
 
-//applet:IF_DATE(APPLET(date, BB_DIR_BIN, BB_SUID_DROP))
+//applet:IF_DATE(APPLET_NOEXEC(date, date, BB_DIR_BIN, BB_SUID_DROP, date))
 
 //kbuild:lib-$(CONFIG_DATE) += date.o
 
@@ -152,12 +152,6 @@ enum {
        OPT_HINT      = (1 << 6) * ENABLE_FEATURE_DATE_ISOFMT, /* D */
 };
 
-static void maybe_set_utc(int opt)
-{
-       if (opt & OPT_UTC)
-               putenv((char*)"TZ=UTC0");
-}
-
 #if ENABLE_LONG_OPTS
 static const char date_longopts[] ALIGN1 =
                "rfc-822\0"   No_argument       "R"
@@ -170,6 +164,19 @@ static const char date_longopts[] ALIGN1 =
                ;
 #endif
 
+/* We are a NOEXEC applet.
+ * Obstacles to NOFORK:
+ * - we change env
+ * - xasprintf result not freed
+ * - after xasprintf we use other xfuncs
+ */
+
+static void maybe_set_utc(int opt)
+{
+       if (opt & OPT_UTC)
+               putenv((char*)"TZ=UTC0");
+}
+
 int date_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int date_main(int argc UNUSED_PARAM, char **argv)
 {

diff --git a/coreutils/readlink.c b/coreutils/readlink.c
index 9690290e3c3ecdd9a1d758b3e91a81e132396154..7f8d6b2397a924e8dccc83d4be4c3ece02ce8045 100644 (file)
--- a/coreutils/readlink.c
+++ b/coreutils/readlink.c
@@ -20,7 +20,7 @@
 //config:      help
 //config:      Enable the readlink option (-f).
 
-//applet:IF_READLINK(APPLET(readlink, BB_DIR_USR_BIN, BB_SUID_DROP))
+//applet:IF_READLINK(APPLET_NOFORK(readlink, readlink, BB_DIR_USR_BIN, BB_SUID_DROP, readlink))
 
 //kbuild:lib-$(CONFIG_READLINK) += readlink.o
 
@@ -85,6 +85,7 @@ int readlink_main(int argc UNUSED_PARAM, char **argv)
        if (!(opt & 4)) /* not -v */
                logmode = LOGMODE_NONE;
 
+       /* NOFORK: only one alloc is allowed; must free */
        if (opt & 1) { /* -f */
                buf = xmalloc_realpath(fname);
        } else {
@@ -94,9 +95,7 @@ int readlink_main(int argc UNUSED_PARAM, char **argv)
        if (!buf)
                return EXIT_FAILURE;
        printf((opt & 2) ? "%s" : "%s\n", buf);
-
-       if (ENABLE_FEATURE_CLEAN_UP)
-               free(buf);
+       free(buf);
 
        fflush_stdout_and_exit(EXIT_SUCCESS);
 }

diff --git a/coreutils/realpath.c b/coreutils/realpath.c
index 6a61c3dc880c92cb6a7d49e11f9c79e6ac321d42..f9c630135b34c1f2dbfe47fd901acb28d0e45f75 100644 (file)
--- a/coreutils/realpath.c
+++ b/coreutils/realpath.c
@@ -13,7 +13,7 @@
 //config:      Return the canonicalized absolute pathname.
 //config:      This isn't provided by GNU shellutils, but where else does it belong.
 
-//applet:IF_REALPATH(APPLET(realpath, BB_DIR_USR_BIN, BB_SUID_DROP))
+//applet:IF_REALPATH(APPLET_NOFORK(realpath, realpath, BB_DIR_USR_BIN, BB_SUID_DROP, realpath))
 
 //kbuild:lib-$(CONFIG_REALPATH) += realpath.o
 
@@ -36,6 +36,7 @@ int realpath_main(int argc UNUSED_PARAM, char **argv)
        }
 
        do {
+               /* NOFORK: only one alloc is allowed; must free */
                char *resolved_path = xmalloc_realpath(*argv);
                if (resolved_path != NULL) {
                        puts(resolved_path);

diff --git a/docs/nofork_noexec.txt b/docs/nofork_noexec.txt
index 0ad4e6e605bfa31caf31916d9552ab6b179ce9c1..cfb02145a447e87d670d866d1f32d5aea3e16d89 100644 (file)
--- a/docs/nofork_noexec.txt
+++ b/docs/nofork_noexec.txt
@@ -98,7 +98,7 @@ It itself calls run_nofork_applet(), if argv[0] turned out to be a name
 of a NOFORK applet.
 
 run_nofork_applet() saves/inits/restores option parsing, xfunc_error_retval,
-applet_name. Thus, for example, caller does not need to worry about
+logmode, applet_name. Thus, for example, caller does not need to worry about
 option_mask32 getting trashed.
 
 

diff --git a/libbb/bb_pwd.c b/libbb/bb_pwd.c
index 4829b723a0195163df5e937090d4c171ab9d8460..dca0a150bea3fb1bfddb15d6303706cd793f1a2f 100644 (file)
--- a/libbb/bb_pwd.c
+++ b/libbb/bb_pwd.c
@@ -31,9 +31,9 @@ struct group* FAST_FUNC xgetgrnam(const char *name)
        return gr;
 }
 
-
 struct passwd* FAST_FUNC xgetpwuid(uid_t uid)
 {
+       /* Note: used in nofork applets (whoami), be careful not to leak anything */
        struct passwd *pw = getpwuid(uid);
        if (!pw)
                bb_error_msg_and_die("unknown uid %u", (unsigned)uid);
@@ -50,6 +50,7 @@ struct group* FAST_FUNC xgetgrgid(gid_t gid)
 
 char* FAST_FUNC xuid2uname(uid_t uid)
 {
+       /* Note: used in nofork applets (whoami), be careful not to leak anything */
        struct passwd *pw = xgetpwuid(uid);
        return pw->pw_name;
 }

diff --git a/libbb/vfork_daemon_rexec.c b/libbb/vfork_daemon_rexec.c
index 576534ee5d5eca3698a93cfeeccb36527d3e5a27..487ecb0e4ffeb4b03e45bb01efe644b5765b04e5 100644 (file)
--- a/libbb/vfork_daemon_rexec.c
+++ b/libbb/vfork_daemon_rexec.c
@@ -92,6 +92,7 @@ struct nofork_save_area {
        void (*die_func)(void);
        const char *applet_name;
        uint32_t option_mask32;
+       smallint logmode;
        uint8_t xfunc_error_retval;
 };
 static void save_nofork_data(struct nofork_save_area *save)
@@ -100,6 +101,7 @@ static void save_nofork_data(struct nofork_save_area *save)
        save->die_func = die_func;
        save->applet_name = applet_name;
        save->option_mask32 = option_mask32;
+       save->logmode = logmode;
        save->xfunc_error_retval = xfunc_error_retval;
 }
 static void restore_nofork_data(struct nofork_save_area *save)
@@ -108,6 +110,7 @@ static void restore_nofork_data(struct nofork_save_area *save)
        die_func = save->die_func;
        applet_name = save->applet_name;
        option_mask32 = save->option_mask32;
+       logmode = save->logmode;
        xfunc_error_retval = save->xfunc_error_retval;
 }
 
@@ -118,8 +121,8 @@ int FAST_FUNC run_nofork_applet(int applet_no, char **argv)
 
        save_nofork_data(&old);
 
+       logmode = LOGMODE_STDIO;
        xfunc_error_retval = EXIT_FAILURE;
-
        /* In case getopt() or getopt32() was already called:
         * reset the libc getopt() function, which keeps internal state.
         */
@@ -146,7 +149,6 @@ int FAST_FUNC run_nofork_applet(int applet_no, char **argv)
 
        /* Restoring some globals */
        restore_nofork_data(&old);
-
        /* Other globals can be simply reset to defaults */
        GETOPT_RESET();
 

diff --git a/miscutils/ttysize.c b/miscutils/ttysize.c
index 7f6a84308075ef37d80b796f9087c72be5adbece..2c2d4ec332220e041bf3d87f48422a4cafab3583 100644 (file)
--- a/miscutils/ttysize.c
+++ b/miscutils/ttysize.c
@@ -18,7 +18,7 @@
 //config:      error, but returns default 80x24.
 //config:      Usage in shell scripts: width=`ttysize w`.
 
-//applet:IF_TTYSIZE(APPLET(ttysize, BB_DIR_USR_BIN, BB_SUID_DROP))
+//applet:IF_TTYSIZE(APPLET_NOFORK(ttysize, ttysize, BB_DIR_USR_BIN, BB_SUID_DROP, ttysize))
 
 //kbuild:lib-$(CONFIG_TTYSIZE) += ttysize.o
 

diff --git a/procps/kill.c b/procps/kill.c
index 09beefb2d378771d28e8c88277e05ab0f56f062d..0ddae2f70811dc74930168cb26aba337921e03b2 100644 (file)
--- a/procps/kill.c
+++ b/procps/kill.c
@@ -32,10 +32,10 @@
 //config:      in its own session, so it won't kill the shell that is running
 //config:      the script it was called from.
 
-//applet:IF_KILL(APPLET(kill, BB_DIR_BIN, BB_SUID_DROP))
-//                   APPLET_ODDNAME:name      main  location         suid_type     help
-//applet:IF_KILLALL( APPLET_ODDNAME(killall,  kill, BB_DIR_USR_BIN,  BB_SUID_DROP, killall))
-//applet:IF_KILLALL5(APPLET_ODDNAME(killall5, kill, BB_DIR_USR_SBIN, BB_SUID_DROP, killall5))
+//applet:IF_KILL(    APPLET_NOFORK(kill,     kill, BB_DIR_BIN,      BB_SUID_DROP, kill))
+//                   APPLET_NOFORK:name      main  location         suid_type     help
+//applet:IF_KILLALL( APPLET_NOFORK(killall,  kill, BB_DIR_USR_BIN,  BB_SUID_DROP, killall))
+//applet:IF_KILLALL5(APPLET_NOFORK(killall5, kill, BB_DIR_USR_SBIN, BB_SUID_DROP, killall5))
 
 //kbuild:lib-$(CONFIG_KILL) += kill.o
 //kbuild:lib-$(CONFIG_KILLALL) += kill.o
@@ -87,7 +87,7 @@
  * + we can't use xfunc here
  * + we can't use applet_name
  * + we can't use bb_show_usage
- * (Above doesn't apply for killall[5] cases)
+ * (doesn't apply for killall[5], still should be careful b/c NOFORK)
  *
  * kill %n gets translated into kill ' -<process group>' by shell (note space!)
  * This is needed to avoid collision with kill -9 ... syntax

diff --git a/procps/pwdx.c b/procps/pwdx.c
index dac238950f0f391ec3464d4e422e805e5e97e20a..84802bbcd5b83c7a289493c9aea619472d53a1ac 100644 (file)
--- a/procps/pwdx.c
+++ b/procps/pwdx.c
@@ -14,7 +14,7 @@
 //config:      help
 //config:      Report current working directory of a process
 
-//applet:IF_PWDX(APPLET(pwdx, BB_DIR_USR_BIN, BB_SUID_DROP))
+//applet:IF_PWDX(APPLET_NOFORK(pwdx, pwdx, BB_DIR_USR_BIN, BB_SUID_DROP, pwdx))
 
 //kbuild:lib-$(CONFIG_PWDX) += pwdx.o
 
@@ -50,6 +50,7 @@ int pwdx_main(int argc UNUSED_PARAM, char **argv)
 
                sprintf(buf, "/proc/%u/cwd", pid);
 
+               /* NOFORK: only one alloc is allowed; must free */
                s = xmalloc_readlink(buf);
                // "pwdx /proc/1" says "/proc/1: DIR", not "1: DIR"
                printf("%s: %s\n", *argv, s ? s : strerror(errno == ENOENT ? ESRCH : errno));