From patchwork Sat Dec 15 02:53:50 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: DJ Delorie X-Patchwork-Id: 30678 Received: (qmail 115088 invoked by alias); 15 Dec 2018 02:53:57 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 115077 invoked by uid 89); 15 Dec 2018 02:53:56 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Date: Fri, 14 Dec 2018 21:53:50 -0500 Message-Id: From: DJ Delorie To: libc-alpha@sourceware.org Subject: shell-container - fd leaks and error checks These three BZs were related enough that I'm grouping the fix (and a bunch of related fixes and cleanups) in one patch. I have a test for the leaks but there's a leak in test-container also, so I'll post the test seperately. [BZ #23987] [BZ #23988] [BZ #23991] * support/shell-container.c (copy_func): Close sfd, dfd, on error exit. Check chmod return. (run_command_array): Close stdin/stdout/stderr on error exit; avoid leaks. Check dup2() return. Fix typo. (run_script): Don't leak descriptor. (main): Check argc before using argv. Print usage if needed. diff --git a/support/shell-container.c b/support/shell-container.c index 9bd90d3f60..01daff410c 100644 --- a/support/shell-container.c +++ b/support/shell-container.c @@ -116,6 +116,7 @@ copy_func (char **argv) { fprintf (stderr, "cp: unable to open %s for writing: %s\n", dname, strerror (errno)); + close (sfd); return 1; } @@ -123,13 +124,20 @@ copy_func (char **argv) { fprintf (stderr, "cp: cannot copy file %s to %s: %s\n", sname, dname, strerror (errno)); + close (sfd); + close (dfd); return 1; } close (sfd); close (dfd); - chmod (dname, st.st_mode & 0777); + if (chmod (dname, st.st_mode & 0777) < 0) + { + fprintf (stderr, "cannot chmod file %s to 0%o: %s\n", + dname, st.st_mode & 0777, strerror (errno)); + return 1; + } return 0; @@ -173,25 +181,73 @@ run_command_array (char **argv) { if (strcmp (argv[i], "<") == 0 && argv[i + 1]) { - new_stdin = open (argv[i + 1], O_WRONLY|O_CREAT|O_TRUNC, 0777); + if (new_stdin != 0) + close (new_stdin); + new_stdin = open (argv[i + 1], O_RDONLY); + if (new_stdin < 0) + { + fprintf (stderr, "%s: cannot open %s for reading: %s\n", + argv[0], argv[i + 1], strerror (errno)); + if (new_stdout != 1) + close (new_stdout); + if (new_stderr != 2) + close (new_stderr); + exit (1); + } ++i; continue; } if (strcmp (argv[i], ">") == 0 && argv[i + 1]) { - new_stdout = open (argv[i + 1], O_WRONLY|O_CREAT|O_TRUNC, 0777); + if (new_stdout != 1) + close (new_stdout); + new_stdout = open (argv[i + 1], O_WRONLY | O_CREAT | O_TRUNC, 0666); + if (new_stdout < 0) + { + fprintf (stderr, "%s: cannot open %s for writing: %s\n", + argv[0], argv[i + 1], strerror (errno)); + if (new_stdin != 0) + close (new_stdin); + if (new_stderr != 2) + close (new_stderr); + exit (1); + } ++i; continue; } if (strcmp (argv[i], ">>") == 0 && argv[i + 1]) { - new_stdout = open (argv[i + 1], O_WRONLY|O_CREAT|O_APPEND, 0777); + if (new_stdout != 1) + close (new_stdout); + new_stdout = open (argv[i + 1], O_WRONLY | O_CREAT | O_APPEND, 0666); + if (new_stdout < 0) + { + fprintf (stderr, "%s: cannot open %s for writing: %s\n", + argv[0], argv[i + 1], strerror (errno)); + if (new_stdin != 0) + close (new_stdin); + if (new_stderr != 2) + close (new_stderr); + exit (1); + } ++i; continue; } if (strcmp (argv[i], "2>") == 0 && argv[i + 1]) { - new_stderr = open (argv[i + 1], O_WRONLY|O_CREAT|O_TRUNC, 0777); + if (new_stderr != 2) + close (new_stderr); + new_stderr = open (argv[i + 1], O_WRONLY | O_CREAT | O_TRUNC, 0666); + if (new_stderr < 0) + { + fprintf (stderr, "%s: cannot open %s for writing: %s\n", + argv[0], argv[i + 1], strerror (errno)); + if (new_stdin != 0) + close (new_stdin); + if (new_stdout != 1) + close (new_stdout); + exit (1); + } ++i; continue; } @@ -210,6 +266,12 @@ run_command_array (char **argv) if (pid < 0) { fprintf (stderr, "sh: fork failed\n"); + if (new_stdin != 0) + close (new_stdin); + if (new_stdout != 1) + close (new_stdout); + if (new_stderr != 2) + close (new_stderr); exit (1); } @@ -217,18 +279,30 @@ run_command_array (char **argv) { if (new_stdin != 0) { - dup2 (new_stdin, 0); + if (dup2 (new_stdin, 0) < 0) + { + perror ("dup2"); + exit (1); + } close (new_stdin); } if (new_stdout != 1) { - dup2 (new_stdout, 1); + if (dup2 (new_stdout, 1) < 0) + { + perror ("dup2"); + exit (1); + } close (new_stdout); } if (new_stderr != 2) { - dup2 (new_stderr, 2); - close (new_stdout); + if (dup2 (new_stderr, 2) < 0) + { + perror ("dup2"); + exit (1); + } + close (new_stderr); } if (builtin_func != NULL) @@ -241,6 +315,13 @@ run_command_array (char **argv) exit (1); } + if (new_stdin != 0) + close (new_stdin); + if (new_stdout != 1) + close (new_stdout); + if (new_stderr != 2) + close (new_stderr); + waitpid (pid, &status, 0); dprintf (stderr, "exiting run_command_array\n"); @@ -351,7 +432,7 @@ run_script (const char *filename, const char **args) { char line[MAX_LINE_LENGTH + 1]; dprintf (stderr, "run_script starting: '%s'\n", filename); - FILE *f = fopen (filename, "r"); + FILE *f = fopen (filename, "re"); if (f == NULL) { fprintf (stderr, "sh: %s: %s\n", filename, strerror (errno)); @@ -374,18 +455,24 @@ main (int argc, const char **argv) { int i; - if (strcmp (argv[1], "--debug") == 0) + if (argc > 1 && strcmp (argv[1], "--debug") == 0) { debug_mode = 1; --argc; ++argv; } + if (argc < 2) + { + fprintf (stderr, "Usage: shell-container [--debug] [-c] command [args...]\n"); + return 1; + } + dprintf (stderr, "container-sh starting:\n"); for (i = 0; i < argc; i++) dprintf (stderr, " argv[%d] is `%s'\n", i, argv[i]); - if (strcmp (argv[1], "-c") == 0) + if (argc > 2 && strcmp (argv[1], "-c") == 0) run_command_string (argv[2], argv+3); else run_script (argv[1], argv+2);