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: 1013810 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-98357-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="OE4npeF4"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43GsSW5r2Zz9s47 for ; Sat, 15 Dec 2018 13:54:03 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:message-id:from:to:subject; q=dns; s= default; b=YTyHKhAa1ttqUplsyfwVM0RXCaf73/QsaPMaXDL+LQx3Ar7wyTBLw /NeuoreaZRAeHp1yw20YZrv7LMDGMXVdESgeatlg4EYNjVo+oourXAPSSiMumehZ L+IyQgJso6yLUBD0H5l9jRAZ8DQ5UfVQhBP8u6HvFU9CcBvtk+lUc4= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:message-id:from:to:subject; s=default; bh=+hIqgOwTE52lRjqXIVIGCJ/vDdA=; b=OE4npeF4Fp4qKEvy4q6xC5Lh1FtZ zT5O1I7b4VJkjmxhfRZJ4pIoJg2nFjoaxn99vDjnYnIORSkWxBKPq4rg0aL1M9qs kNMSxdrWCZg0XdlCZDXAE98Hp/uBxvEWRrqRSrPKX/zHlTaVTZWgHI7/6eX2G1HB tqgPwXQG0fSTwmU= 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);