shell-container - fd leaks and error checks

Message ID xnlg4rmrep.fsf@greed.delorie.com
State New, archived
Headers

Commit Message

DJ Delorie Dec. 15, 2018, 2:53 a.m. UTC
  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.
  

Patch

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);