248a67fb7
Denys Vlasenko
2017-08-07
Subject
free,stat: make NOEXEC
Binary
No binary (build failed)
Architecture & Compiler Flags
—
Build Log
(log yok)
Commit Diff GitHub →
NOFORK_NOEXEC.lst +10 -8coreutils/stat.c +1 -1libbb/vfork_daemon_rexec.c +2 -0procps/free.c +5 -2procps/pgrep.c +5 -1procps/pidof.c +4 -0shell/ash.c +2 -0shell/hush.c +2 -0
NOFORK_NOEXEC.lst+10 -8
| @@ -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 | |
coreutils/stat.c+1 -1
| @@ -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 | |
libbb/vfork_daemon_rexec.c+2 -0
| @@ -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 */ | |
procps/free.c+5 -2
| @@ -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) | |
procps/pgrep.c+5 -1
| @@ -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 | |
procps/pidof.c+4 -0
| @@ -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 | |
shell/ash.c+2 -0
| @@ -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 */ | |
shell/hush.c+2 -0
| @@ -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 | |