[PATCH] xargs: reopen stdin as /dev/null for spawned processes

[Azat Khuzhin]

Otherwise child processes can consume xargs input:

    $ yes /etc/issue | head -n1000 | xargs -i cat | wc -l
    0
    $ yes /etc/issue | head -n1000 | ./busybox xargs -i cat | wc -l
    628

After this patch (comparing busybox to xargs):

    $ yes /etc/issue | head -n1000 | ./busybox xargs cat | wc -l
    2000
    $ yes /etc/issue | head -n1000 | xargs cat | wc -l
    2000

    $ yes /etc/issue | head -n1000 | ./busybox xargs -i cat | wc -l
    0
    $ yes /etc/issue | head -n1000 | xargs -i cat | wc -l
    0

Refs: https://savannah.gnu.org/bugs/?3992
Signed-off-by: Azat Khuzhin <a3at.mail at gmail.com>
---
 findutils/xargs.c     | 50 +++++++++++++++++++++++++++++++------------
 testsuite/xargs.tests | 19 ++++++++++++++++
 2 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/findutils/xargs.c b/findutils/xargs.c
index 890c37534..8105a2043 100644
--- a/findutils/xargs.c
+++ b/findutils/xargs.c
@@ -108,11 +108,19 @@ struct globals {
 	char **argv;
 	const char *repl_str;
 	char eol_ch;
+#endif
+#if ENABLE_FEATURE_XARGS_SUPPORT_ARGS_FILE
+	char *opt_a;
 #endif
 	const char *eof_str;
 	int idx;
 	int fd_tty;
+	/* Used to replace stdin for spawned commands, to forbid access to xargs stdin */
+	int fd_null;
+	/* Used to restore "stdin" (a.k.a. input file with commands) after command spawning */
 	int fd_stdin;
+	/* Original stdin of xargs program, that will be used for spawned programs in case of -a */
+	int fd_stdin_orig;
 #if ENABLE_FEATURE_XARGS_SUPPORT_PARALLEL
 	int running_procs;
 	int max_procs;
@@ -132,6 +140,7 @@ struct globals {
 	setup_common_bufsiz(); \
 	IF_FEATURE_XARGS_SUPPORT_REPL_STR(G.repl_str = "{}";) \
 	IF_FEATURE_XARGS_SUPPORT_REPL_STR(G.eol_ch = '\n';) \
+	IF_FEATURE_XARGS_SUPPORT_ARGS_FILE(G.opt_a = NULL;) \
 	/* Even zero values are set because we are NOEXEC applet */ \
 	G.eof_str = NULL; \
 	G.idx = 0; \
@@ -189,8 +198,16 @@ static int xargs_exec(void)
 {
 	int status;
 
-	if (option_mask32 & OPT_STDIN_TTY)
-		xdup2(G.fd_tty, STDIN_FILENO);
+	int stdin_fd = G.fd_stdin_orig;
+	if (option_mask32 & OPT_STDIN_TTY) {
+		stdin_fd = G.fd_tty;
+	} else {
+#if ENABLE_FEATURE_XARGS_SUPPORT_ARGS_FILE
+		if (!G.opt_a)
+			stdin_fd = G.fd_null;
+#endif
+	}
+	xdup2(stdin_fd, STDIN_FILENO);
 
 #if !ENABLE_FEATURE_XARGS_SUPPORT_PARALLEL
 	status = spawn_and_wait(G.args);
@@ -278,8 +295,7 @@ static int xargs_exec(void)
  ret:
 	if (status != 0)
 		G.xargs_exitcode = status;
-	if (option_mask32 & OPT_STDIN_TTY)
-		xdup2(G.fd_stdin, STDIN_FILENO);
+	xdup2(G.fd_stdin, STDIN_FILENO);
 	return status;
 }
 
@@ -624,8 +640,6 @@ int xargs_main(int argc UNUSED_PARAM, char **argv)
 #else
 #define read_args process_stdin
 #endif
-	IF_FEATURE_XARGS_SUPPORT_ARGS_FILE(char *opt_a = NULL;)
-
 	INIT_G();
 
 	opt = getopt32long(argv, OPTION_STR,
@@ -633,7 +647,7 @@ int xargs_main(int argc UNUSED_PARAM, char **argv)
 		&max_args, &max_chars, &G.eof_str, &G.eof_str
 		IF_FEATURE_XARGS_SUPPORT_REPL_STR(, &G.repl_str, &G.repl_str)
 		IF_FEATURE_XARGS_SUPPORT_PARALLEL(, &G.max_procs)
-		IF_FEATURE_XARGS_SUPPORT_ARGS_FILE(, &opt_a)
+		IF_FEATURE_XARGS_SUPPORT_ARGS_FILE(, &G.opt_a)
 	);
 
 #if ENABLE_FEATURE_XARGS_SUPPORT_PARALLEL
@@ -641,11 +655,6 @@ int xargs_main(int argc UNUSED_PARAM, char **argv)
 		G.max_procs = 100; /* let's not go crazy high */
 #endif
 
-#if ENABLE_FEATURE_XARGS_SUPPORT_ARGS_FILE
-	if (opt_a)
-		xmove_fd(xopen(opt_a, O_RDONLY), 0);
-#endif
-
 	/* -E ""? You may wonder why not just omit -E?
 	 * This is used for portability:
 	 * old xargs was using "_" as default for -E / -e */
@@ -736,11 +745,24 @@ int xargs_main(int argc UNUSED_PARAM, char **argv)
 			store_param(argv[i]);
 	}
 
+	/* Preserve original stdin to reopen it for child in case of -a */
+	G.fd_stdin_orig = dup(STDIN_FILENO);
+	close_on_exec_on(G.fd_stdin_orig);
+
+#if ENABLE_FEATURE_XARGS_SUPPORT_ARGS_FILE
+	if (G.opt_a)
+		xmove_fd(xopen(G.opt_a, O_RDONLY), STDIN_FILENO);
+#endif
+
+	G.fd_stdin = dup(STDIN_FILENO);
+	close_on_exec_on(G.fd_stdin);
+
 	if (opt & OPT_STDIN_TTY) {
 		G.fd_tty = xopen(CURRENT_TTY, O_RDONLY);
 		close_on_exec_on(G.fd_tty);
-		G.fd_stdin = dup(STDIN_FILENO);
-		close_on_exec_on(G.fd_stdin);
+	} else {
+		G.fd_null = xopen("/dev/null", O_RDONLY);
+		close_on_exec_on(G.fd_null);
 	}
 
 	initial_idx = G.idx;
diff --git a/testsuite/xargs.tests b/testsuite/xargs.tests
index c5e7b99e9..77d7e8b2e 100755
--- a/testsuite/xargs.tests
+++ b/testsuite/xargs.tests
@@ -59,6 +59,25 @@ testing "xargs -n2" \
 	"1 2\n3 4\n5\n" \
 	"" "1 2 3 4 5\n"
 
+# ensure that programs that is run under xargs
+# will not have access to xargs stdin
+optional FEATURE_XARGS_SUPPORT_REPL_STR
+testing "xargs closes stdin" \
+	"xargs -i cat | wc -l" \
+	"0\n" \
+	"" \
+	"$(yes /etc/issue | head -n1000)"
+
+# ensure that cat will get original stdin in case of xargs has -a, so here it
+# will got "yes /etc/issue | head -n10", but due to stripping whitespaces it
+# will got only 9 lines
+optional FEATURE_XARGS_SUPPORT_REPL_STR FEATURE_XARGS_SUPPORT_ARGS_FILE
+testing "xargs closes stdin with -a" \
+	"xargs -a input -i cat | wc -l" \
+	"9\n" \
+	"/etc/issue" \
+	"$(yes /etc/issue | head -n10)"
+
 SKIP=
 
 optional FEATURE_XARGS_SUPPORT_QUOTES FEATURE_XARGS_SUPPORT_REPL_STR
-- 
2.47.1