[git commit] new NOFORKs: pwdx, kill[all5], ttysize, realpath, readlink NOEXECs: date, resize
Denys Vlasenko
vda.linux at googlemail.com
Thu Aug 3 17:03:32 UTC 2017
commit: https://git.busybox.net/busybox/commit/?id=39194f030918b87eeb3e11e94cfa05f575fb47b4
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master
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 at googlemail.com>
---
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 02eba46..7073611 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 62928a0..9786667 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 2c6e1d4..89b2816 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 9690290..7f8d6b2 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 6a61c3d..f9c6301 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 0ad4e6e..cfb0214 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 4829b72..dca0a15 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 576534e..487ecb0 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 7f6a843..2c2d4ec 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 09beefb..0ddae2f 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 ' -<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 dac2389..84802bb 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));
More information about the busybox-cvs
mailing list