From 39194f030918b87eeb3e11e94cfa05f575fb47b4 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Thu, 3 Aug 2017 19:00:01 +0200 Subject: [PATCH] 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 --- NOFORK_NOEXEC.lst | 140 +++++++++++++++++++------------------ console-tools/resize.c | 2 +- coreutils/date.c | 21 ++++-- coreutils/readlink.c | 7 +- coreutils/realpath.c | 3 +- docs/nofork_noexec.txt | 2 +- libbb/bb_pwd.c | 3 +- libbb/vfork_daemon_rexec.c | 6 +- miscutils/ttysize.c | 2 +- procps/kill.c | 10 +-- procps/pwdx.c | 3 +- 11 files changed, 106 insertions(+), 93 deletions(-) diff --git a/NOFORK_NOEXEC.lst b/NOFORK_NOEXEC.lst index 02eba46e8..7073611a4 100644 --- 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 62928a01e..97866673a 100644 --- 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 2c6e1d4df..89b281646 100644 --- 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 9690290e3..7f8d6b239 100644 --- 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 6a61c3dc8..f9c630135 100644 --- 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 0ad4e6e60..cfb02145a 100644 --- 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 4829b723a..dca0a150b 100644 --- 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 576534ee5..487ecb0e4 100644 --- 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 7f6a84308..2c2d4ec33 100644 --- 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 09beefb2d..0ddae2f70 100644 --- 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 ' -' 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 dac238950..84802bbcd 100644 --- 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)); -- 2.25.1