[PATCH v5 8/9] applets: use BB_EXECVP and BB_EXECVPE instead of exec calls

Nadav Tasher tashernadav at gmail.com
Wed Jan 29 23:36:19 UTC 2025


This replaces all invocations of execs with BB_EXECVP(E).
It provides better control over executed programs and allows
all applets to seamlessly execute other applets instead of
just calling exec.

Signed-off-by: Nadav Tasher <tashernadav at gmail.com>
---
 console-tools/reset.c           |  2 +-
 debianutils/start_stop_daemon.c |  2 +-
 init/bootchartd.c               | 17 +++++++++++----
 init/halt.c                     |  9 ++------
 libbb/run_shell.c               |  2 +-
 loginutils/adduser.c            |  7 ++++--
 loginutils/getty.c              |  8 +++++--
 miscutils/conspy.c              | 11 ++++++++--
 miscutils/crond.c               | 38 ++++++++++++++++++++++++++++-----
 miscutils/crontab.c             | 13 +++++++++--
 networking/ftpd.c               |  3 +--
 networking/ifupdown.c           | 15 ++++++++++++-
 runit/runsv.c                   |  9 ++++++--
 runit/runsvdir.c                | 13 ++++++++++-
 runit/svlogd.c                  | 15 ++++++++++++-
 util-linux/script.c             | 16 ++++++++++++--
 util-linux/switch_root.c        |  2 +-
 17 files changed, 145 insertions(+), 37 deletions(-)

diff --git a/console-tools/reset.c b/console-tools/reset.c
index 655a5ef7a..b2f34a1eb 100644
--- a/console-tools/reset.c
+++ b/console-tools/reset.c
@@ -58,7 +58,7 @@ int reset_main(int argc UNUSED_PARAM, char **argv UNUSED_PARAM)
 #else
 		/* Make sure stdout gets drained before we execvp */
 		fflush_all();
-		execvp("stty", (char**)args);
+		BB_EXECVP("stty", (char**)args);
 #endif
 	}
 	return EXIT_SUCCESS;
diff --git a/debianutils/start_stop_daemon.c b/debianutils/start_stop_daemon.c
index 271bc4edf..4b42b23a8 100644
--- a/debianutils/start_stop_daemon.c
+++ b/debianutils/start_stop_daemon.c
@@ -629,6 +629,6 @@ int start_stop_daemon_main(int argc UNUSED_PARAM, char **argv)
 	 * strace -oLOG start-stop-daemon -S -x /bin/usleep -a qwerty 500000
 	 * should exec "/bin/usleep", but argv[0] should be "qwerty":
 	 */
-	execvp(execname, argv);
+	BB_EXECVP(execname, argv);
 	bb_perror_msg_and_die("can't execute '%s'", startas);
 }
diff --git a/init/bootchartd.c b/init/bootchartd.c
index f23025fbf..149f9da34 100644
--- a/init/bootchartd.c
+++ b/init/bootchartd.c
@@ -445,11 +445,20 @@ int bootchartd_main(int argc UNUSED_PARAM, char **argv)
 	}
 
 	if (cmd == CMD_PID1) {
+		char init_arg0[] = "init", *init_argv[2] = {init_arg0, NULL};
+
 		char *bootchart_init = getenv("bootchart_init");
-		if (bootchart_init)
-			execl(bootchart_init, bootchart_init, NULL);
-		execl("/init", "init", NULL);
-		execl("/sbin/init", "init", NULL);
+		if (bootchart_init) {
+			char *bootchart_arg0 = xstrdup(bootchart_init), *bootchart_argv[2] = {bootchart_arg0, NULL};
+			BB_EXECVP(bootchart_arg0, bootchart_argv);
+
+			/* free copied argument */
+			free(bootchart_arg0);
+		}
+
+		/* fallback, we are calling different init binaries */
+		BB_EXECVP("/init", init_argv);
+		BB_EXECVP("/sbin/init", init_argv);
 		bb_perror_msg_and_die("can't execute '%s'", "/sbin/init");
 	}
 
diff --git a/init/halt.c b/init/halt.c
index 7aea8cfec..6b0aabb2e 100644
--- a/init/halt.c
+++ b/init/halt.c
@@ -230,13 +230,8 @@ int halt_main(int argc UNUSED_PARAM, char **argv)
 				/* runlevels:
 				 * 0 == shutdown
 				 * 6 == reboot */
-				execlp(CONFIG_TELINIT_PATH,
-						CONFIG_TELINIT_PATH,
-						which == 2 ? "6" : "0",
-						(char *)NULL
-				);
-				bb_perror_msg_and_die("can't execute '%s'",
-						CONFIG_TELINIT_PATH);
+				char telinit_arg0[] = CONFIG_TELINIT_PATH, telinit_arg1_6[] = "6", telinit_arg1_0[] = "0", *telinit_argv[] = {telinit_arg0, which == 2 ? telinit_arg1_6 : telinit_arg1_0, NULL};
+				BB_EXECVP_or_die(telinit_argv);
 			}
 		}
 	} else {
diff --git a/libbb/run_shell.c b/libbb/run_shell.c
index c22bba87b..99184ebe9 100644
--- a/libbb/run_shell.c
+++ b/libbb/run_shell.c
@@ -80,7 +80,7 @@ void FAST_FUNC exec_shell(const char *shell, int loginshell, const char **additi
 	if (ENABLE_FEATURE_CLEAN_UP)
 		freecon(current_sid);
 #endif
-	execv(shell, (char **) args);
+	BB_EXECVP(shell, (char **) args);
 	bb_perror_msg_and_die("can't execute '%s'", shell);
 }
 
diff --git a/loginutils/adduser.c b/loginutils/adduser.c
index d3c795afa..cc3cb4572 100644
--- a/loginutils/adduser.c
+++ b/loginutils/adduser.c
@@ -158,8 +158,11 @@ static void passwd_wrapper(const char *login_name) NORETURN;
 
 static void passwd_wrapper(const char *login_name)
 {
-	BB_EXECLP("passwd", "passwd", "--", login_name, NULL);
-	bb_simple_error_msg_and_die("can't execute passwd, you must set password manually");
+	char passwd_arg0[] = "passwd", passwd_arg1[] = "--", *passwd_arg2 = xstrdup(login_name), *passwd_argv[] = {passwd_arg0, passwd_arg1, passwd_arg2, NULL};
+	BB_EXECVP_or_die_msg(passwd_argv, "can't execute '%s', you must set password manually");
+
+	/* free copied arguments */
+	free(passwd_arg2);
 }
 
 //FIXME: upstream adduser has no short options! NOT COMPATIBLE!
diff --git a/loginutils/getty.c b/loginutils/getty.c
index 4581cc9f7..36cd7ba6a 100644
--- a/loginutils/getty.c
+++ b/loginutils/getty.c
@@ -732,6 +732,10 @@ int getty_main(int argc UNUSED_PARAM, char **argv)
 	/* We use PATH because we trust that root doesn't set "bad" PATH,
 	 * and getty is not suid-root applet */
 	/* With -n, logname == NULL, and login will ask for username instead */
-	BB_EXECLP(G.login, G.login, "--", logname, (char *)0);
-	bb_error_msg_and_die("can't execute '%s'", G.login);
+	char *login_arg0 = xstrdup(G.login), login_arg1[] = "--", *login_arg2 = xstrdup(logname), *login_argv[] = {login_arg0, login_arg1, login_arg2, NULL};
+	BB_EXECVP_or_die(login_argv);
+
+	/* free copied arguments */
+	free(login_arg0);
+	free(login_arg2);
 }
diff --git a/miscutils/conspy.c b/miscutils/conspy.c
index 21a498d0f..b670e688e 100644
--- a/miscutils/conspy.c
+++ b/miscutils/conspy.c
@@ -332,6 +332,7 @@ static void create_cdev_if_doesnt_exist(const char* name, dev_t dev)
 
 static NOINLINE void start_shell_in_child(const char* tty_name)
 {
+	char *shell_arg0, shell_arg1[] = "-i", *shell_argv[3];
 	int pid = xvfork();
 	if (pid == 0) {
 		struct termios termchild;
@@ -353,8 +354,14 @@ static NOINLINE void start_shell_in_child(const char* tty_name)
 		termchild.c_iflag |= ICRNL;
 		termchild.c_iflag &= ~IXOFF;
 		tcsetattr_stdin_TCSANOW(&termchild);
-		execl(shell, shell, "-i", (char *) NULL);
-		bb_simple_perror_msg_and_die(shell);
+		// duplicate shell, so it is safe
+		shell_arg0 = xstrdup(shell);
+		shell_argv[0] = shell_arg0;
+		shell_argv[1] = shell_arg1;
+		shell_argv[2] = NULL;
+		BB_EXECVP_or_die(shell_argv);
+		// free copied argument
+		free(shell_arg0);
 	}
 }
 
diff --git a/miscutils/crond.c b/miscutils/crond.c
index b3762d327..8eb53b309 100644
--- a/miscutils/crond.c
+++ b/miscutils/crond.c
@@ -699,6 +699,8 @@ fork_job(const char *user, int mailFd, CronLine *line, bool run_sendmail)
 	const char *shell, *prog;
 	smallint sv_logmode;
 	pid_t pid;
+	char *shell_arg0, shell_arg1[] = "-c", *shell_arg2, *shell_argv[4];
+	char sendmail_arg0[] = SENDMAIL, sendmail_arg1[] = SENDMAIL_ARGS, *sendmail_argv[] = {sendmail_arg0, sendmail_arg1, NULL};
 
 	/* prepare things before vfork */
 	pas = getpwnam(user);
@@ -725,10 +727,22 @@ fork_job(const char *user, int mailFd, CronLine *line, bool run_sendmail)
 		}
 		/* crond 3.0pl1-100 puts tasks in separate process groups */
 		bb_setpgrp();
-		if (!run_sendmail)
-			execlp(prog, prog, "-c", line->cl_cmd, (char *) NULL);
-		else
-			execlp(prog, prog, SENDMAIL_ARGS, (char *) NULL);
+		if (!run_sendmail) {
+			shell_arg0 = xstrdup(shell);
+			shell_arg2 = xstrdup(line->cl_cmd);
+
+			shell_argv[0] = shell_arg0;
+			shell_argv[1] = shell_arg1;
+			shell_argv[2] = shell_arg2;
+			shell_argv[3] = NULL;
+
+			BB_EXECVP(shell_argv[0], shell_argv);
+
+			free(shell_arg0);
+			free(shell_arg2);
+		} else {
+			BB_EXECVP(sendmail_argv[0], sendmail_argv);
+		}
 		/*
 		 * I want this error message on stderr too,
 		 * even if other messages go only to syslog:
@@ -845,6 +859,7 @@ static pid_t start_one_job(const char *user, CronLine *line)
 	const char *shell;
 	struct passwd *pas;
 	pid_t pid;
+	char *shell_arg0, shell_arg1[] = "-c", *shell_arg2, *shell_argv[4];
 
 	pas = getpwnam(user);
 	if (!pas) {
@@ -865,7 +880,20 @@ static pid_t start_one_job(const char *user, CronLine *line)
 		log5("child running %s", shell);
 		/* crond 3.0pl1-100 puts tasks in separate process groups */
 		bb_setpgrp();
-		execl(shell, shell, "-c", line->cl_cmd, (char *) NULL);
+
+		shell_arg0 = xstrdup(shell);
+		shell_arg2 = xstrdup(line->cl_cmd);
+
+		shell_argv[0] = shell_arg0;
+		shell_argv[1] = shell_arg1;
+		shell_argv[2] = shell_arg2;
+		shell_argv[3] = NULL;
+
+		BB_EXECVP(shell_argv[0], shell_argv);
+
+		free(shell_arg0);
+		free(shell_arg2);
+
 		bb_error_msg_and_die("can't execute '%s' for user %s", shell, user);
 	}
 	if (pid < 0) {
diff --git a/miscutils/crontab.c b/miscutils/crontab.c
index 1111f4d54..17b255504 100644
--- a/miscutils/crontab.c
+++ b/miscutils/crontab.c
@@ -44,6 +44,7 @@ static void edit_file(const struct passwd *pas, const char *file)
 {
 	const char *ptr;
 	pid_t pid;
+	char *edit_arg0, *edit_arg1, *edit_argv[3];
 
 	pid = xvfork();
 	if (pid) { /* parent */
@@ -64,8 +65,16 @@ static void edit_file(const struct passwd *pas, const char *file)
 			ptr = "vi";
 	}
 
-	BB_EXECLP(ptr, ptr, file, NULL);
-	bb_perror_msg_and_die("can't execute '%s'", ptr);
+	edit_arg0 = xstrdup(ptr);
+	edit_arg1 = xstrdup(file);
+	
+	edit_argv[0] = edit_arg0;
+	edit_argv[1] = edit_arg1;
+	edit_argv[2] = NULL;
+	BB_EXECVP_or_die(edit_argv);
+
+	free(edit_arg0);
+	free(edit_arg1);
 }
 
 int crontab_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
diff --git a/networking/ftpd.c b/networking/ftpd.c
index 0d6a289c7..075f3b143 100644
--- a/networking/ftpd.c
+++ b/networking/ftpd.c
@@ -733,8 +733,7 @@ popen_ls(const char *opt)
 		}
 		/* Child expects directory to list on fd #3 */
 		xmove_fd(cur_fd, 3);
-		execv(bb_busybox_exec_path, (char**) argv);
-		_exit(127);
+		BB_EXECVP_or_die(argv);
 #endif
 	}
 
diff --git a/networking/ifupdown.c b/networking/ifupdown.c
index 9c3640be7..0c8254933 100644
--- a/networking/ifupdown.c
+++ b/networking/ifupdown.c
@@ -1146,6 +1146,7 @@ static void set_environ(struct interface_defn_t *iface, const char *mode, const
 
 static int doit(char *str)
 {
+	char *shell_arg0, shell_arg1[] = "-c", *shell_arg2, *shell_argv[4];
 	if (option_mask32 & (OPT_no_act|OPT_verbose)) {
 		puts(str);
 	}
@@ -1158,7 +1159,19 @@ static int doit(char *str)
 		if (child < 0) /* failure */
 			return 0;
 		if (child == 0) { /* child */
-			execle(G.shell, G.shell, "-c", str, (char *) NULL, G.my_environ);
+			shell_arg0 = xstrdup(G.shell);
+			shell_arg2 = xstrdup(str);
+
+			shell_argv[0] = shell_arg0;
+			shell_argv[1] = shell_arg1;
+			shell_argv[2] = shell_arg2;
+			shell_argv[3] = NULL;
+
+			BB_EXECVPE(shell_argv[0], shell_argv, G.my_environ);
+
+			free(shell_arg0);
+			free(shell_arg1);
+
 			_exit(127);
 		}
 		safe_waitpid(child, &status, 0);
diff --git a/runit/runsv.c b/runit/runsv.c
index 20a445319..abb88afc4 100644
--- a/runit/runsv.c
+++ b/runit/runsv.c
@@ -280,6 +280,7 @@ static unsigned custom(struct svdir *s, char c)
 	int w;
 	char a[10];
 	struct stat st;
+	char *exec_argv[2];
 
 	if (s->islog)
 		return 0;
@@ -296,7 +297,11 @@ static unsigned custom(struct svdir *s, char c)
 				/* child */
 				if (haslog && dup2(logpipe.wr, 1) == -1)
 					warn2_cannot("setup stdout for ", a);
-				execl(a, a, (char *) NULL);
+
+				exec_argv[0] = a;
+				exec_argv[1] = NULL;
+
+				BB_EXECVP(exec_argv[0], exec_argv);
 				fatal2_cannot("run ", a);
 			}
 			/* parent */
@@ -391,7 +396,7 @@ static void startservice(struct svdir *s)
 		signal(SIGTERM, SIG_DFL);
 		sig_unblock(SIGCHLD);
 		sig_unblock(SIGTERM);
-		execv(arg[0], (char**) arg);
+		BB_EXECVP(arg[0], (char**) arg);
 		fatal2_cannot(s->islog ? "start log/" : "start ", arg[0]);
 	}
 	/* parent */
diff --git a/runit/runsvdir.c b/runit/runsvdir.c
index d6629dedd..e57d8bd53 100644
--- a/runit/runsvdir.c
+++ b/runit/runsvdir.c
@@ -119,6 +119,7 @@ static void warnx(const char *m1)
 static NOINLINE pid_t runsv(const char *name)
 {
 	pid_t pid;
+	char runsv_arg0[] = "runsv", *runsv_arg1, *runsv_argv[3];
 
 	/* If we got signaled, stop spawning children at once! */
 	if (bb_got_signal)
@@ -143,7 +144,17 @@ static NOINLINE pid_t runsv(const char *name)
 			| (1 << SIGTERM)
 			, SIG_DFL);
 #endif
-		execlp("runsv", "runsv", name, (char *) NULL);
+		/* we have to keep name safe */
+		runsv_arg1 = xstrdup(name);
+		
+		runsv_argv[0] = runsv_arg0;
+		runsv_argv[1] = runsv_arg1;
+		runsv_argv[2] = NULL;
+
+		BB_EXECVP(runsv_argv[0], runsv_argv);
+		
+		free(runsv_arg1);
+
 		fatal2_cannot("start runsv ", name);
 	}
 	return pid;
diff --git a/runit/svlogd.c b/runit/svlogd.c
index f7576f0fa..df8a0c465 100644
--- a/runit/svlogd.c
+++ b/runit/svlogd.c
@@ -393,6 +393,7 @@ static void processorstart(struct logdir *ld)
 {
 	char sv_ch;
 	int pid;
+	char *shell_arg0, shell_arg1[] = "-c", *shell_arg2, *shell_argv[4];
 
 	if (!ld->processor) return;
 	if (ld->ppid) {
@@ -453,7 +454,19 @@ static void processorstart(struct logdir *ld)
 		fd = xopen("newstate", O_WRONLY|O_NDELAY|O_TRUNC|O_CREAT);
 		xmove_fd(fd, 5);
 
-		execl(G.shell, G.shell, "-c", ld->processor, (char*) NULL);
+		shell_arg0 = xstrdup(G.shell);
+		shell_arg2 = xstrdup(ld->processor);
+
+		shell_argv[0] = shell_arg0;
+		shell_argv[1] = shell_arg1;
+		shell_argv[2] = shell_arg2;
+		shell_argv[3] = NULL;
+
+		BB_EXECVP(shell_argv[0], shell_argv);
+
+		free(shell_arg0);
+		free(shell_arg2);
+
 		bb_perror_msg_and_die(FATAL"can't %s processor %s", "run", ld->name);
 	}
 	ld->fnsave[26] = sv_ch; /* ...restore */
diff --git a/util-linux/script.c b/util-linux/script.c
index 58b844e77..e824b19c6 100644
--- a/util-linux/script.c
+++ b/util-linux/script.c
@@ -60,8 +60,10 @@ int script_main(int argc UNUSED_PARAM, char **argv)
 	const char *str_t = NULL;
 	const char *fname = "typescript";
 	const char *shell;
+	char *shell_copy;
 	char shell_opt[] = "-i";
 	char *shell_arg = NULL;
+	char *shell_argv[4];
 	enum {
 		OPT_a = (1 << 0),
 		OPT_c = (1 << 1),
@@ -235,6 +237,16 @@ int script_main(int argc UNUSED_PARAM, char **argv)
 
 	/* Non-ignored signals revert to SIG_DFL on exec anyway */
 	/*signal(SIGCHLD, SIG_DFL);*/
-	execl(shell, shell, shell_opt, shell_arg, (char *) NULL);
-	bb_simple_perror_msg_and_die(shell);
+
+	/* copy shell argument, we don't want it to be modified. */
+	shell_copy = xstrdup(shell);
+
+	shell_argv[0] = shell_copy;
+	shell_argv[1] = shell_opt;
+	shell_argv[2] = shell_arg;
+	shell_argv[3] = NULL;
+	BB_EXECVP_or_die(shell_argv);
+
+	/* free copied shell */
+	free(shell_copy);
 }
diff --git a/util-linux/switch_root.c b/util-linux/switch_root.c
index 14139736e..ec65b35ca 100644
--- a/util-linux/switch_root.c
+++ b/util-linux/switch_root.c
@@ -281,7 +281,7 @@ int switch_root_main(int argc UNUSED_PARAM, char **argv)
 			return 0;
 	} else {
 		// Exec NEW_INIT
-		execv(argv[0], argv);
+		BB_EXECVP(argv[0], argv);
 	}
 	bb_perror_msg_and_die("can't execute '%s'", argv[0]);
 }
-- 
2.43.0



More information about the busybox mailing list