822039917

Denys Vlasenko

2016-04-02

SUCCESS Duration: 272s

Subject

nsenter,unshare: share common code; fix a bug of not closing all fds

No binary (build failed)

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

(log yok)

include/libbb.h+4 -2

@@ -992,9 +992,10 @@ int BB_EXECVP(const char file, char const argv[]) FAST_FUNC;
	#define BB_EXECVP(prog,cmd) execvp(prog,cmd)
	#define BB_EXECLP(prog,cmd,...) execlp(prog,cmd,__VA_ARGS__)
	#endif
-	int BB_EXECVP_or_die(char **argv) NORETURN FAST_FUNC;
+	void BB_EXECVP_or_die(char **argv) NORETURN FAST_FUNC;
+	void exec_prog_or_SHELL(char **argv) NORETURN FAST_FUNC;

-	/* xvfork() can't be a _function_, return after vfork mangles stack
+	/* xvfork() can't be a _function_, return after vfork in child mangles stack
	* in the parent. It must be a macro. */
	#define xvfork() \
	({ \
@@ -1006,6 +1007,7 @@ int BB_EXECVP_or_die(char **argv) NORETURN FAST_FUNC;
	#if BB_MMU
	pid_t xfork(void) FAST_FUNC;
	#endif
+	void xvfork_parent_waits_and_exits(void) FAST_FUNC;

	/* NOMMU friendy fork+exec: */
	pid_t spawn(char **argv) FAST_FUNC;

libbb/executable.c+10 -1

@@ -83,10 +83,19 @@ int FAST_FUNC BB_EXECVP(const char file, char const argv[])
	}
	#endif

-	int FAST_FUNC BB_EXECVP_or_die(char **argv)
+	void FAST_FUNC BB_EXECVP_or_die(char **argv)
	{
	BB_EXECVP(argv[0], argv);
	/* SUSv3-mandated exit codes */
	xfunc_error_retval = (errno == ENOENT) ? 127 : 126;
	bb_perror_msg_and_die("can't execute '%s'", argv[0]);
	}
+
+	/* Typical idiom for applets which exec optional PROG [ARGS] */
+	void FAST_FUNC exec_prog_or_SHELL(char **argv)
+	{
+	if (argv[0]) {
+	BB_EXECVP_or_die(argv);
+	}
+	run_shell(getenv("SHELL"), /login:/ 1, NULL, NULL);
+	}

libbb/xfuncs_printf.c+16 -0

@@ -659,3 +659,19 @@ pid_t FAST_FUNC xfork(void)
	return pid;
	}
	#endif
+
+	void FAST_FUNC xvfork_parent_waits_and_exits(void)
+	{
+	pid_t pid;
+
+	fflush_all();
+	pid = xvfork();
+	if (pid > 0) {
+	/* Parent */
+	int exit_status = wait_for_exitstatus(pid);
+	if (WIFSIGNALED(exit_status))
+	kill_myself_with_sig(WTERMSIG(exit_status));
+	_exit(WEXITSTATUS(exit_status));
+	}
+	/* Child continues */
+	}

util-linux/nsenter.c+5 -16

@@ -230,7 +230,7 @@ int nsenter_main(int argc UNUSED_PARAM, char **argv)
	ns->ns_nsfile8 + 3 /* skip over "ns/" */
	);
	}
-	/close(ns_ctx->fd);/
+	close(ns_ctx->fd); /* should close fds, to not confuse exec'ed PROG */
	/ns_ctx->fd = -1;/
	}

@@ -244,13 +244,13 @@ int nsenter_main(int argc UNUSED_PARAM, char **argv)
	}
	xfchdir(root_fd);
	xchroot(".");
-	/close(root_fd);/
+	close(root_fd);
	/root_fd = -1;/
	}

	if (wd_fd >= 0) {
	xfchdir(wd_fd);
-	/close(wd_fd);/
+	close(wd_fd);
	/wd_fd = -1;/
	}

@@ -259,14 +259,7 @@ int nsenter_main(int argc UNUSED_PARAM, char **argv)
	* explicitly requested by the user not to.
	*/
	if (!(opts & OPT_nofork) && (opts & OPT_pid)) {
-	pid_t pid = xvfork();
-	if (pid > 0) {
-	/* Parent */
-	int exit_status = wait_for_exitstatus(pid);
-	if (WIFSIGNALED(exit_status))
-	kill_myself_with_sig(WTERMSIG(exit_status));
-	return WEXITSTATUS(exit_status);
-	}
+	xvfork_parent_waits_and_exits();
	/* Child continues */
	}

@@ -278,9 +271,5 @@ int nsenter_main(int argc UNUSED_PARAM, char **argv)
	if (opts & OPT_setuid)
	xsetuid(uid);

-	if (*argv) {
-	BB_EXECVP_or_die(argv);
-	}
-
-	run_shell(getenv("SHELL"), /login:/ 1, NULL, NULL);
+	exec_prog_or_SHELL(argv);
	}

util-linux/unshare.c+3 -16

@@ -281,7 +281,7 @@ int unshare_main(int argc UNUSED_PARAM, char **argv)

	if (fdp.wr >= 0) {
	close(fdp.wr); /* Release child */
-	/close(fdp.rd);/
+	close(fdp.rd); /* should close fd, to not confuse exec'ed PROG */
	}

	if (need_mount) {
@@ -307,14 +307,7 @@ int unshare_main(int argc UNUSED_PARAM, char **argv)
	* that'll become PID 1 in this new namespace.
	*/
	if (opts & OPT_fork) {
-	pid_t pid = xfork();
-	if (pid > 0) {
-	/* Parent */
-	int exit_status = wait_for_exitstatus(pid);
-	if (WIFSIGNALED(exit_status))
-	kill_myself_with_sig(WTERMSIG(exit_status));
-	return WEXITSTATUS(exit_status);
-	}
+	xvfork_parent_waits_and_exits();
	/* Child continues */
	}

@@ -354,11 +347,5 @@ int unshare_main(int argc UNUSED_PARAM, char **argv)
	mount_or_die("proc", proc_mnt_target, "proc", MS_NOSUID \| MS_NOEXEC \| MS_NODEV);
	}

-	if (argv[0]) {
-	BB_EXECVP_or_die(argv);
-	}
-	/* unshare from util-linux 2.27.1, despite not documenting it,
-	* runs a login shell (argv0="-sh") if no PROG is given
-	*/
-	run_shell(getenv("SHELL"), /login:/ 1, NULL, NULL);
+	exec_prog_or_SHELL(argv);
	}