public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: libc-alpha@sourceware.org
Subject: [PATCH v2 1/3] posix: Remove dynamic memory allocation from execl{e,p}
Date: Mon, 01 Feb 2016 16:21:00 -0000	[thread overview]
Message-ID: <1454343665-1706-2-git-send-email-adhemerval.zanella@linaro.org> (raw)
In-Reply-To: <1454343665-1706-1-git-send-email-adhemerval.zanella@linaro.org>

GLIBC execl{e,p} implementation might use malloc if the total number of i
arguments exceed initial assumption size (1024).  This might lead to
issue in two situations:

1. execl/execle is stated to be async-signal-safe by POSIX [1].  However
   if execl is used in a signal handler with a large argument set (that
   may call malloc internally) and the resulting call fails, it might
   lead malloc in the program in a bad state.

2. If the functions are used in a vfork/clone(VFORK) situation it also
   might issue malloc internal bad state.

This patch fixes it by using stack allocation instead.  It also fixes
BZ#19534.

Tested on x86_64.

Changes from previous version:

- Remove arbitrary limit on stack allocation for argument handling
  (it is arbitrary and does no impose any limit since it does not 
  consider current stack size neither stack size limit).

[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html

	[BZ #19534]
	* posix/execl.c (execl): Remove dynamic memory allocation.
	* posix/execle.c (execle): Likewise.
	* posix/execlp.c (execlp): Likewise.
---
 posix/execl.c  | 65 +++++++++++++++++----------------------------------------
 posix/execle.c | 66 ++++++++++++++++++----------------------------------------
 posix/execlp.c | 64 +++++++++++++++++---------------------------------------
 4 files changed, 65 insertions(+), 137 deletions(-)

diff --git a/posix/execl.c b/posix/execl.c
index 102d19d..8b8a324 100644
--- a/posix/execl.c
+++ b/posix/execl.c
@@ -16,58 +16,31 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include <unistd.h>
+#include <errno.h>
 #include <stdarg.h>
-#include <stddef.h>
-#include <stdlib.h>
-#include <string.h>
-
-#include <stackinfo.h>
-
+#include <sys/param.h>
 
 /* Execute PATH with all arguments after PATH until
    a NULL pointer and environment from `environ'.  */
 int
 execl (const char *path, const char *arg, ...)
 {
-#define INITIAL_ARGV_MAX 1024
-  size_t argv_max = INITIAL_ARGV_MAX;
-  const char *initial_argv[INITIAL_ARGV_MAX];
-  const char **argv = initial_argv;
-  va_list args;
-
-  argv[0] = arg;
-
-  va_start (args, arg);
-  unsigned int i = 0;
-  while (argv[i++] != NULL)
-    {
-      if (i == argv_max)
-	{
-	  argv_max *= 2;
-	  const char **nptr = realloc (argv == initial_argv ? NULL : argv,
-				       argv_max * sizeof (const char *));
-	  if (nptr == NULL)
-	    {
-	      if (argv != initial_argv)
-		free (argv);
-	      va_end (args);
-	      return -1;
-	    }
-	  if (argv == initial_argv)
-	    /* We have to copy the already filled-in data ourselves.  */
-	    memcpy (nptr, argv, i * sizeof (const char *));
-
-	  argv = nptr;
-	}
-
-      argv[i] = va_arg (args, const char *);
-    }
-  va_end (args);
-
-  int ret = __execve (path, (char *const *) argv, __environ);
-  if (argv != initial_argv)
-    free (argv);
-
-  return ret;
+  int argc;
+  va_list ap;
+  va_start (ap, arg);
+  for (argc = 1; va_arg (ap, const char *); argc++)
+    continue;
+  va_end (ap);
+
+  int i;
+  char *argv[argc+1];
+  va_start (ap, arg);
+  argv[0] = (char*) arg;
+  for (i = 1; i < argc; i++)
+     argv[i] = va_arg (ap, char *);
+  argv[i] = NULL;
+  va_end (ap);
+
+  return __execve (path, argv, __environ);
 }
 libc_hidden_def (execl)
diff --git a/posix/execle.c b/posix/execle.c
index 8edc03a..1a0c9ee 100644
--- a/posix/execle.c
+++ b/posix/execle.c
@@ -17,57 +17,31 @@
 
 #include <unistd.h>
 #include <stdarg.h>
-#include <stddef.h>
-#include <stdlib.h>
-#include <string.h>
-
-#include <stackinfo.h>
+#include <errno.h>
+#include <sys/param.h>
 
 /* Execute PATH with all arguments after PATH until a NULL pointer,
    and the argument after that for environment.  */
 int
 execle (const char *path, const char *arg, ...)
 {
-#define INITIAL_ARGV_MAX 1024
-  size_t argv_max = INITIAL_ARGV_MAX;
-  const char *initial_argv[INITIAL_ARGV_MAX];
-  const char **argv = initial_argv;
-  va_list args;
-  argv[0] = arg;
-
-  va_start (args, arg);
-  unsigned int i = 0;
-  while (argv[i++] != NULL)
-    {
-      if (i == argv_max)
-	{
-	  argv_max *= 2;
-	  const char **nptr = realloc (argv == initial_argv ? NULL : argv,
-				       argv_max * sizeof (const char *));
-	  if (nptr == NULL)
-	    {
-	      if (argv != initial_argv)
-		free (argv);
-	      va_end (args);
-	      return -1;
-	    }
-	  if (argv == initial_argv)
-	    /* We have to copy the already filled-in data ourselves.  */
-	    memcpy (nptr, argv, i * sizeof (const char *));
-
-	  argv = nptr;
-	}
-
-      argv[i] = va_arg (args, const char *);
-    }
-
-  const char *const *envp = va_arg (args, const char *const *);
-  va_end (args);
-
-  int ret = __execve (path, (char *const *) argv, (char *const *) envp);
-  if (argv != initial_argv)
-    free (argv);
-
-  return ret;
+  int argc;
+  va_list ap;
+  va_start (ap, arg);
+  for (argc = 1; va_arg (ap, const char *); argc++)
+    continue;
+  va_end (ap);
+
+  int i;
+  char *argv[argc+1];
+  char **envp;
+  va_start (ap, arg);
+  argv[0] = (char*) arg;
+  for (i = 1; i < argc; i++)
+     argv[i] = va_arg (ap, char *);
+  envp = va_arg (ap, char **);
+  va_end (ap);
+
+  return __execve (path, argv, envp);
 }
 libc_hidden_def (execle)
diff --git a/posix/execlp.c b/posix/execlp.c
index 6700994..a0e1859 100644
--- a/posix/execlp.c
+++ b/posix/execlp.c
@@ -17,11 +17,8 @@
 
 #include <unistd.h>
 #include <stdarg.h>
-#include <stddef.h>
-#include <stdlib.h>
-#include <string.h>
-
-#include <stackinfo.h>
+#include <errno.h>
+#include <sys/param.h>
 
 /* Execute FILE, searching in the `PATH' environment variable if
    it contains no slashes, with all arguments after FILE until a
@@ -29,45 +26,22 @@
 int
 execlp (const char *file, const char *arg, ...)
 {
-#define INITIAL_ARGV_MAX 1024
-  size_t argv_max = INITIAL_ARGV_MAX;
-  const char *initial_argv[INITIAL_ARGV_MAX];
-  const char **argv = initial_argv;
-  va_list args;
-
-  argv[0] = arg;
-
-  va_start (args, arg);
-  unsigned int i = 0;
-  while (argv[i++] != NULL)
-    {
-      if (i == argv_max)
-	{
-	  argv_max *= 2;
-	  const char **nptr = realloc (argv == initial_argv ? NULL : argv,
-				       argv_max * sizeof (const char *));
-	  if (nptr == NULL)
-	    {
-	      if (argv != initial_argv)
-		free (argv);
-	      va_end (args);
-	      return -1;
-	    }
-	  if (argv == initial_argv)
-	    /* We have to copy the already filled-in data ourselves.  */
-	    memcpy (nptr, argv, i * sizeof (const char *));
-
-	  argv = nptr;
-	}
-
-      argv[i] = va_arg (args, const char *);
-    }
-  va_end (args);
-
-  int ret = execvp (file, (char *const *) argv);
-  if (argv != initial_argv)
-    free (argv);
-
-  return ret;
+  int argc;
+  va_list ap;
+  va_start (ap, arg);
+  for (argc = 1; va_arg (ap, const char *); argc++)
+    continue;
+  va_end (ap);
+
+  int i;
+  char *argv[argc+1];
+  va_start (ap, arg);
+  argv[0] = (char*) arg;
+  for (i = 1; i < argc; i++)
+     argv[i] = va_arg (ap, char *);
+  argv[i] = NULL;
+  va_end (ap);
+
+  return __execvpe (file, argv, __environ);
 }
 libc_hidden_def (execlp)
-- 
1.9.1

  parent reply	other threads:[~2016-02-01 16:21 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-01 16:21 [PATCH 0/3] posix: Execute file function fixes Adhemerval Zanella
2016-02-01 16:21 ` [PATCH v2 3/3] posix: New Linux posix_spawn{p} implementation Adhemerval Zanella
2016-02-02 13:06   ` Florian Weimer
2016-02-02 13:31     ` Adhemerval Zanella
2016-02-03 11:07       ` Torvald Riegel
2016-02-03 12:05         ` Adhemerval Zanella
2016-02-03 12:06           ` Torvald Riegel
2016-02-03 12:14             ` Adhemerval Zanella
2016-08-31 21:13   ` Rasmus Villemoes
2016-08-31 22:08     ` Joseph Myers
2016-09-01  9:28       ` Rasmus Villemoes
2016-09-14 13:13         ` Adhemerval Zanella
2016-09-14 18:58           ` Rasmus Villemoes
2016-09-14 19:59             ` Adhemerval Zanella
2016-09-20 20:25         ` Florian Weimer
2016-09-20 20:54           ` Rasmus Villemoes
2016-09-20 21:01           ` [PATCH] linux: spawni.c: simplify error reporting to parent Rasmus Villemoes
2016-09-22 20:54             ` Adhemerval Zanella
2016-09-23  5:21             ` Florian Weimer
2016-09-23 19:09               ` Rasmus Villemoes
2016-09-23 19:28                 ` Adhemerval Zanella
2016-09-23 20:37                   ` Florian Weimer
2016-09-27 20:26                     ` Rasmus Villemoes
2016-09-27 21:14                       ` Adhemerval Zanella
2016-09-28 14:14             ` Rich Felker
2016-09-28 15:03               ` Andreas Schwab
2016-09-28 15:22                 ` Rich Felker
2016-09-28 18:13                   ` Adhemerval Zanella
2016-10-06 12:52                     ` Florian Weimer
2016-09-06 20:43     ` [PATCH v2 3/3] posix: New Linux posix_spawn{p} implementation Rasmus Villemoes
2016-09-14 19:37     ` Adhemerval Zanella
2016-02-01 16:21 ` [PATCH v4 2/3] posix: execvpe cleanup Adhemerval Zanella
2016-02-01 16:21 ` Adhemerval Zanella [this message]
2016-02-01 16:52   ` [PATCH v2 1/3] posix: Remove dynamic memory allocation from execl{e,p} Joseph Myers
2016-02-01 17:18     ` Adhemerval Zanella
2016-02-01 17:54       ` Joseph Myers
2016-02-01 18:09         ` Adhemerval Zanella
2016-02-02 11:24       ` Florian Weimer
2016-02-02 12:46         ` Szabolcs Nagy
2016-02-02 12:47         ` Adhemerval Zanella
2016-02-02 16:33     ` Rich Felker
2016-02-07 21:28       ` Rasmus Villemoes
2016-02-09 11:36         ` Richard Henderson
2016-02-09 13:30           ` Szabolcs Nagy
2016-02-10 16:36             ` Adhemerval Zanella

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1454343665-1706-2-git-send-email-adhemerval.zanella@linaro.org \
    --to=adhemerval.zanella@linaro.org \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).