free,stat: make NOEXEC
authorDenys Vlasenko <vda.linux@googlemail.com>
Mon, 7 Aug 2017 16:18:09 +0000 (18:18 +0200)
committerDenys Vlasenko <vda.linux@googlemail.com>
Mon, 7 Aug 2017 16:18:09 +0000 (18:18 +0200)
pkill/pgrep/pidof uncovered another quirk: what about noexec's _process names_?

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
NOFORK_NOEXEC.lst
coreutils/stat.c
libbb/vfork_daemon_rexec.c
procps/free.c
procps/pgrep.c
procps/pidof.c
shell/ash.c
shell/hush.c

index 70f38d867f13e5ede55908c357a2a6a921dc260d..8ec3bdbe621d56a9f98d3db1fbc6cc5bddb1a8f2 100644 (file)
@@ -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
index 3b85808b5d7dbb57f0f6f2e3eca4965f0a4e3dc0..4e926a9087ca8c7ea666101b263dbc147f5a5450 100644 (file)
@@ -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
 
index f84e678b5fd5e4e9bb46eddc56aa4bcfb8e7c01b..50ecea762ead413e48d4c3d27dacb08ae5982aa9 100644 (file)
@@ -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 */
index 618664e082d1e502fe320f4cc2a96b308396bf1d..b57e4a322054c9597dce2c21a6684bc96246cc89 100644 (file)
@@ -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)
index a3ca9e295686495f0bf328cf8c255e46de363661..a16a6e959ccce1b6ebd075d780ec3dda24350126 100644 (file)
 //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
index 41247a02c722f1bc553c37b7fa3f7cbace91b77e..98d7949f825f56184f833025722a5e1f65edbd8d 100644 (file)
 //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
 
index e8f3ed26be7535ea90be37993bcd90fb781c7ff8..0a323e957fa83514500194d528bc9098bbceadb9 100644 (file)
@@ -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 */
index bb80f422c11bee893c82a8a03ea6b29ed01a159d..b4fe7146b528cfb8fd11a44cb84440db58f2d03e 100644 (file)
@@ -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