From 248a67fb75a0d2c98f4f9935b7bb9e11382b2c78 Mon Sep 17 00:00:00 2001
From: Denys Vlasenko <vda.linux@googlemail.com>
Date: Mon, 7 Aug 2017 18:18:09 +0200
Subject: free,stat: make NOEXEC
pkill/pgrep/pidof uncovered another quirk: what about noexec's _process names_?
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
---
NOFORK_NOEXEC.lst | 18 ++++++++++--------
coreutils/stat.c | 2 +-
libbb/vfork_daemon_rexec.c | 2 ++
procps/free.c | 7 +++++--
procps/pgrep.c | 6 +++++-
procps/pidof.c | 4 ++++
shell/ash.c | 2 ++
shell/hush.c | 2 ++
8 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/NOFORK_NOEXEC.lst b/NOFORK_NOEXEC.lst
index 70f38d867..8ec3bdbe6 100644
--- a/NOFORK_NOEXEC.lst
+++ b/NOFORK_NOEXEC.lst
@@ -16,6 +16,8 @@ leak categories.
Why can't be NOEXEC:
suid: runs under different uid - must fork+exec
+if it's important that /proc/PID/cmdline and comm are correct.
+ ("pkill sh" killing itself before it kills real "sh" is no fun)
Why shouldn't be NOFORK/NOEXEC:
rare: not started often enough to bother optimizing (example: poweroff)
@@ -131,7 +133,7 @@ flash_unlock - hardware
flashcp - hardware
flock - spawner, changes state (file locks), let's play safe and not be noexec
fold - noexec. runner
-free - nofork candidate(struct globals, needs to close /proc/meminfo fd)
+free - noexec. nofork candidate(struct globals, needs to close /proc/meminfo fd)
freeramdisk - leaks: open+ioctl_or_perror_and_die
fsck - interactive, longterm
fsck.minix - needs ^C
@@ -172,7 +174,7 @@ inotifyd - daemon
insmod - noexec
install - runner
ionice - noexec. spawner
-iostat - runner
+iostat - longterm: "iostat 1" runs indefinitely
ip - noexec candidate
ipaddr - noexec candidate
ipcalc - noexec candidate
@@ -244,7 +246,7 @@ mv - noexec candidate, runner
nameif - noexec. openlog(), leaks: config_open2+ioctl_or_perror_and_die
nbd-client - noexec
nc - runner
-netstat - runner with -c
+netstat - longterm with -c (continuous listing)
nice - noexec. spawner
nl - runner
nmeter - longterm
@@ -257,13 +259,13 @@ partprobe - noexec. leaks: open+ioctl_or_perror_and_die(BLKRRPART)
passwd - suid
paste - noexec. runner
patch - needs ^C
-pgrep - nofork candidate(xregcomp, procps_scan - are they ok?)
-pidof - nofork candidate(uses find_pid_by_name, is that ok?)
+pgrep - must fork+exec to get correct /proc/PID/cmdline and comm field
+pidof - must fork+exec to get correct /proc/PID/cmdline and comm field
ping - suid, longterm
ping6 - suid, longterm
pipe_progress - longterm
pivot_root - NOFORK
-pkill - nofork candidate(xregcomp, procps_scan - are they ok?)
+pkill - must fork+exec to get correct /proc/PID/cmdline and comm field
pmap - noexec candidate, leaks: open+xstrdup
popmaildir - runner
poweroff - rare
@@ -329,7 +331,7 @@ sort - noexec. runner
split - runner
ssl_client - longterm
start-stop-daemon - not noexec: uses bb_common_bufsiz1
-stat - nofork candidate(needs fewer allocs)
+stat - noexec. nofork candidate(needs fewer allocs)
strings - runner
stty - noexec. nofork candidate: has no allocs or opens except xmove_fd(xopen("-F DEVICE"),STDIN). tcsetattr(STDIN) is not a problem: it would work the same across processes sharing this fd
su - suid, spawner
@@ -338,7 +340,7 @@ sum - runner
sv - noexec. needs ^C (uses usleep(420000))
svc - noexec. needs ^C (uses usleep(420000))
svlogd - daemon
-swapoff - rare
+swapoff - longterm: may cause memory pressure, execing is beneficial
swapon - rare
switch_root - spawner, rare, changes state (oh yes), execing may be important to free binary's inode
sync - NOFORK
diff --git a/coreutils/stat.c b/coreutils/stat.c
index 3b85808b5..4e926a908 100644
--- a/coreutils/stat.c
+++ b/coreutils/stat.c
@@ -36,7 +36,7 @@
//config: Without this, stat will not support the '-f' option to display
//config: information about filesystem status.
-//applet:IF_STAT(APPLET(stat, BB_DIR_BIN, BB_SUID_DROP))
+//applet:IF_STAT(APPLET_NOEXEC(stat, stat, BB_DIR_BIN, BB_SUID_DROP, stat))
//kbuild:lib-$(CONFIG_STAT) += stat.o
diff --git a/libbb/vfork_daemon_rexec.c b/libbb/vfork_daemon_rexec.c
index f84e678b5..50ecea762 100644
--- a/libbb/vfork_daemon_rexec.c
+++ b/libbb/vfork_daemon_rexec.c
@@ -175,6 +175,8 @@ int FAST_FUNC spawn_and_wait(char **argv)
return wait4pid(rc);
/* child */
+//TODO: prctl(PR_SET_NAME, (long)argv[0], 0, 0, 0);? [think pidof, pgrep, pkill]
+//Rewrite /proc/PID/cmdline? (need to save argv0 and length at init for this to work!)
/* reset some state and run without execing */
/* msg_eol = "\n"; - no caller needs this reinited yet */
diff --git a/procps/free.c b/procps/free.c
index 618664e08..b57e4a322 100644
--- a/procps/free.c
+++ b/procps/free.c
@@ -15,7 +15,7 @@
//config: memory in the system, as well as the buffers used by the kernel.
//config: The shared memory column should be ignored; it is obsolete.
-//applet:IF_FREE(APPLET(free, BB_DIR_USR_BIN, BB_SUID_DROP))
+//applet:IF_FREE(APPLET_NOEXEC(free, free, BB_DIR_USR_BIN, BB_SUID_DROP, free))
//kbuild:lib-$(CONFIG_FREE) += free.o
@@ -47,7 +47,10 @@ struct globals {
#endif
} FIX_ALIASING;
#define G (*(struct globals*)bb_common_bufsiz1)
-#define INIT_G() do { setup_common_bufsiz(); } while (0)
+#define INIT_G() do { \
+ setup_common_bufsiz(); \
+ /* NB: noexec applet - globals not zeroed */ \
+} while (0)
static unsigned long long scale(unsigned long d)
diff --git a/procps/pgrep.c b/procps/pgrep.c
index a3ca9e295..a16a6e959 100644
--- a/procps/pgrep.c
+++ b/procps/pgrep.c
@@ -18,9 +18,13 @@
//config: help
//config: Send signals to processes by name.
-//applet:IF_PGREP(APPLET(pgrep, BB_DIR_USR_BIN, BB_SUID_DROP))
+//applet:IF_PGREP(APPLET_ODDNAME(pgrep, pgrep, BB_DIR_USR_BIN, BB_SUID_DROP, pgrep))
// APPLET_ODDNAME:name main location suid_type help
//applet:IF_PKILL(APPLET_ODDNAME(pkill, pgrep, BB_DIR_USR_BIN, BB_SUID_DROP, pkill))
+/* can't be noexec: can find _itself_ under wrong name, since after fork only,
+ * /proc/PID/cmdline and comm are wrong! Can fix comm (prctl(PR_SET_NAME)),
+ * but cmdline?
+ */
//kbuild:lib-$(CONFIG_PGREP) += pgrep.o
//kbuild:lib-$(CONFIG_PKILL) += pgrep.o
diff --git a/procps/pidof.c b/procps/pidof.c
index 41247a02c..98d7949f8 100644
--- a/procps/pidof.c
+++ b/procps/pidof.c
@@ -30,6 +30,10 @@
//config: of the pidof, in other words the calling shell or shell script.
//applet:IF_PIDOF(APPLET(pidof, BB_DIR_BIN, BB_SUID_DROP))
+/* can't be noexec: can find _itself_ under wrong name, since after fork only,
+ * /proc/PID/cmdline and comm are wrong! Can fix comm (prctl(PR_SET_NAME)),
+ * but cmdline?
+ */
//kbuild:lib-$(CONFIG_PIDOF) += pidof.o
diff --git a/shell/ash.c b/shell/ash.c
index e8f3ed26b..0a323e957 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -7803,6 +7803,8 @@ tryexec(IF_FEATURE_SH_STANDALONE(int applet_no,) const char *cmd, char **argv, c
while (*envp)
putenv(*envp++);
popredir(/*drop:*/ 1);
+//TODO: prctl(PR_SET_NAME, (long)argv[0], 0, 0, 0);? [think pidof, pgrep, pkill]
+//Rewrite /proc/PID/cmdline? (need to save argv0 and length at init for this to work!)
run_applet_no_and_exit(applet_no, cmd, argv);
}
/* re-exec ourselves with the new arguments */
diff --git a/shell/hush.c b/shell/hush.c
index bb80f422c..b4fe7146b 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -7387,6 +7387,8 @@ static NOINLINE void pseudo_exec_argv(nommu_save_t *nommu_save,
/* 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]);
+//TODO: prctl(PR_SET_NAME, (long)argv[0], 0, 0, 0);? [think pidof, pgrep, pkill]
+//Rewrite /proc/PID/cmdline? (need to save argv0 and length at init for this to work!)
run_applet_no_and_exit(a, argv[0], argv);
}
# endif
--
cgit v1.2.3