From 63d9049cb83245dfcda22faab2301d75b0aec7e1 Mon Sep 17 00:00:00 2001 From: Eric Bollengier Date: Wed, 7 Sep 2022 11:26:21 +0200 Subject: [PATCH] Fix #9443 About incorrect management of the STDERR via open_bpipe() --- bacula/src/lib/bpipe.c | 11 +++ bacula/src/lib/lex.c | 3 +- bacula/src/tools/bpipe-test.c | 112 +++++++++++++++++++++++++++++ bacula/src/win32/compat/compat.cpp | 16 ++++- 4 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 bacula/src/tools/bpipe-test.c diff --git a/bacula/src/lib/bpipe.c b/bacula/src/lib/bpipe.c index 4840111be..4eb05d442 100644 --- a/bacula/src/lib/bpipe.c +++ b/bacula/src/lib/bpipe.c @@ -56,6 +56,7 @@ int num_execvp_errors = (int)(sizeof(execvp_errors)/sizeof(int)); #define MODE_WRITE 2 #define MODE_SHELL 4 #define MODE_STDERR 8 +#define MODE_NOSTDERR 16 #if !defined(HAVE_WIN32) static void build_argc_argv(char *cmd, int *bargc, char *bargv[], int max_arg); @@ -102,6 +103,7 @@ BPIPE *open_bpipe(char *prog, int wait, const char *mode, char *envp[]) if (strchr(mode,'w')) mode_map|=MODE_WRITE; if (strchr(mode,'s')) mode_map|=MODE_SHELL; if (strchr(mode,'e')) mode_map|=MODE_STDERR; + if (strchr(mode,'E')) mode_map|=MODE_NOSTDERR; /* Build arguments for running program. */ tprog = get_pool_memory(PM_FNAME); @@ -209,7 +211,16 @@ BPIPE *open_bpipe(char *prog, int wait, const char *mode, char *envp[]) if (mode_map & MODE_STDERR) { /* and handle stderr */ close(errp[0]); dup2(errp[1], 2); + + } else if (mode_map & MODE_NOSTDERR) { + /* We open STDERR and map it to /dev/null directly */ + int fd = open("/dev/null", O_WRONLY); + if (fd != 2) { + dup2(fd, 2); + close(fd); + } } else { + /* by default STDERR and STDOUT are merged */ dup2(readp[1], 2); } } diff --git a/bacula/src/lib/lex.c b/bacula/src/lib/lex.c index cf22ae7f9..c7ff1914b 100644 --- a/bacula/src/lib/lex.c +++ b/bacula/src/lib/lex.c @@ -242,11 +242,10 @@ LEX *lex_open_file(LEX *lf, const char *filename, LEX_ERROR_HANDLER *scan_error) char *fname = bstrdup(filename); if (fname[0] == '|') { - if ((bpipe = open_bpipe(fname+1, 0, "reb")) == NULL) { + if ((bpipe = open_bpipe(fname+1, 0, "rEb")) == NULL) { free(fname); return NULL; } - close_epipe(bpipe); /* discard stderr messages */ fd = bpipe->rfd; } else if ((fd = fopen(fname, "rb")) == NULL) { free(fname); diff --git a/bacula/src/tools/bpipe-test.c b/bacula/src/tools/bpipe-test.c new file mode 100644 index 000000000..b6e0f97fc --- /dev/null +++ b/bacula/src/tools/bpipe-test.c @@ -0,0 +1,112 @@ +/* + Bacula(R) - The Network Backup Solution + + Copyright (C) 2000-2023 Kern Sibbald + + The original author of Bacula is Kern Sibbald, with contributions + from many others, a complete list can be found in the file AUTHORS. + + You may use this file and others of this release according to the + license defined in the LICENSE file, which includes the Affero General + Public License, v3.0 ("AGPLv3") and some additional permissions and + terms pursuant to its AGPLv3 Section 7. + + This notice must be preserved when any source code is + conveyed and/or propagated. + + Bacula(R) is a registered trademark of Kern Sibbald. +*/ + +#include "bacula.h" +#include "lib/unittests.h" + +char *run_cmd(char *command, POOLMEM **out) +{ + **out=0; + if (run_program_full_output(command, 0, *out) != 0) { + berrno be; + Jmsg(NULL, M_ERROR, 0, _("Can't run command %s. ERR=%s\n"), + command, be.bstrerror(errno)); + if (**out) { + Dmsg1(0, "%s", *out); + } + return NULL; + } + + strip_trailing_junk(*out); + + return *out; +} + +void *th1(void *arg) +{ + const char *ret; + POOLMEM *q = get_pool_memory(PM_FNAME); + run_cmd((char *)"echo toto", &q); + if (strcmp(q, "toto") == 0) { + ret = "ok"; + } else { + ret = "not ok"; + } + free_pool_memory(q); + return (void *)ret; +} + +int main(int argc, char **argv) +{ + pthread_t ids[1000]; + void *ret; + Unittests t("bpipe_test", true, true); + debug_level = 400; + //t.configure(TEST_QUIET); + init_stack_dump(); + my_name_is(argc, argv, "bpipe-test"); + init_msg(NULL, NULL); + daemon_start_time = time(NULL); + setup_daemon_message_queue(); + + start_watchdog(); + for (int i = 0 ; i < 990 ; i++) { + pthread_create(&(ids[i]), NULL, th1, NULL); + pthread_create(&(ids[i+1]), NULL, th1, NULL); + pthread_create(&(ids[i+2]), NULL, th1, NULL); + pthread_join(ids[i], &ret); + ok(strcmp((char *)ret, "ok") == 0, "Checking thread output"); + pthread_join(ids[++i], &ret); + ok(strcmp((char *)ret, "ok") == 0, "Checking thread output"); + pthread_join(ids[++i], &ret); + ok(strcmp((char *)ret, "ok") == 0, "Checking thread output"); + } + + FILE *fp = fopen("tmp/a.pl", "w"); + if (fp) { + fprintf(fp, "#!/usr/bin/perl -w\n" + "use strict;\n" + "sleep 1;\n" + "print STDERR \"error\\n\";" + "print \"ok\\n\";\n"); + fclose(fp); + chmod("tmp/a.pl", 0700); + + char buf[512]; + BPIPE *p = open_bpipe((char *)"./tmp/a.pl", 0, "re"); + close_epipe(p); + fgets(buf, sizeof(buf), p->rfd); + int ret = close_bpipe(p); + isnt(ret, 0, "checking bpipe output status (re)"); + is(buf, "ok\n", "checking bpipe output string (re)"); + + p = open_bpipe((char *)"./tmp/a.pl", 0, "rE"); + fgets(buf, sizeof(buf), p->rfd); + ret = close_bpipe(p); + is(ret, 0, "checking bpipe output status (rE)"); + is(buf, "ok\n", "checking bpipe output string (rE)"); + } else { + ok(0, "Unable to open tmp/a.sh for tests"); + } + free_daemon_message_queue(); + stop_watchdog(); + term_msg(); + close_memory_pool(); + return report(); +} diff --git a/bacula/src/win32/compat/compat.cpp b/bacula/src/win32/compat/compat.cpp index 747950baf..77852de91 100644 --- a/bacula/src/win32/compat/compat.cpp +++ b/bacula/src/win32/compat/compat.cpp @@ -2724,6 +2724,7 @@ CloseHandleIfValid(HANDLE handle) #define MODE_WRITE 2 #define MODE_SHELL 4 #define MODE_STDERR 8 +#define MODE_NOSTDERR 16 BPIPE * open_bpipe(char *prog, int wait, const char *mode, char *envp[]) @@ -2750,6 +2751,7 @@ open_bpipe(char *prog, int wait, const char *mode, char *envp[]) if (strchr(mode,'w')) mode_map|=MODE_WRITE; if (strchr(mode,'s')) mode_map|=MODE_SHELL; if (strchr(mode,'e')) mode_map|=MODE_STDERR; + if (strchr(mode,'E')) mode_map|=MODE_NOSTDERR; // Set the bInheritHandle flag so pipe handles are inherited. saAttr.nLength = sizeof(SECURITY_ATTRIBUTES); @@ -2788,7 +2790,6 @@ open_bpipe(char *prog, int wait, const char *mode, char *envp[]) } // Create noninheritable read handle and close the inheritable read // handle. - fSuccess = DuplicateHandle(GetCurrentProcess(), hChildStderrRd, GetCurrentProcess(), &hChildStderrRdDup , 0, FALSE, @@ -2825,12 +2826,20 @@ open_bpipe(char *prog, int wait, const char *mode, char *envp[]) CloseHandle(hChildStdinWr); hChildStdinWr = INVALID_HANDLE_VALUE; } + if (mode_map & MODE_NOSTDERR) { + hChildStderrWr = CreateFileW(L"NUL", GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, NULL); + if (hChildStderrWr == INVALID_HANDLE_VALUE) { + ErrorExit("CreateFile failed"); + goto cleanup; + } + } // spawn program with redirected handles as appropriate bpipe->worker_pid = (pid_t) CreateChildProcess(prog, // commandline hChildStdinRd, // stdin HANDLE hChildStdoutWr, // stdout HANDLE - (mode_map & MODE_STDERR) ? hChildStderrWr:hChildStdoutWr, // stderr HANDLE + (mode_map & MODE_STDERR || mode_map & MODE_NOSTDERR) + ? hChildStderrWr:hChildStdoutWr, // stderr HANDLE envp); // envp environment variables are added to the current process env var set. if ((HANDLE) bpipe->worker_pid == INVALID_HANDLE_VALUE) { @@ -2851,6 +2860,9 @@ open_bpipe(char *prog, int wait, const char *mode, char *envp[]) bpipe->rfd = _fdopen(rfd, "rb"); } } + if (mode_map & MODE_NOSTDERR) { + CloseHandle(hChildStderrWr); // No need to keep it open + } if (mode_map & MODE_STDERR) { CloseHandle(hChildStderrWr); // close our write side so when // process terminates we can -- 2.47.3