[git commit] free,stat: make NOEXEC

Denys Vlasenko vda.linux at googlemail.com
Mon Aug 7 16:18:09 UTC 2017


commit: https://git.busybox.net/busybox/commit/?id=248a67fb75a0d2c98f4f9935b7bb9e11382b2c78
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

pkill/pgrep/pidof uncovered another quirk: what about noexec's _process names_?

Signed-off-by: Denys Vlasenko <vda.linux at 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 70f38d8..8ec3bdb 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 3b85808..4e926a9 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 f84e678..50ecea7 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 618664e..b57e4a3 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 a3ca9e2..a16a6e9 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 41247a0..98d7949 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 e8f3ed2..0a323e9 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 bb80f42..b4fe714 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


More information about the busybox-cvs mailing list