Re: Fw: [PATCH] Improve execl* functions

From a13bf0a67ab85138cd31a6673494f30284fb9b60 Mon Sep 17 00:00:00 2001
From: Terraneo Federico <[email protected]>
Date: Tue, 28 Jan 2025 17:19:18 +0100
Subject: [PATCH 2/2] Lift 256 arg limit in execl, execle and execlp, add
 --enable-newlib-use-malloc-in-execl option

The previous version of these functions allocated a 256 entry array and copied
arguments in that array with no bound checking. That implementation always
occupied 1024 bytes of stack for the array even in the common case in which the
number of passed arguments is far less than 256, risking stack overflows in
environments with small stacks, and caused a stack buffer overflow if called
with more than 256 arguments.

The improved implementation counts the actual number of passed arguments and
allocates a suitable buffer. The default implementation uses alloca to allocate
the buffer to satisfy the POSIX.1-2008 requirement that execl and execle should
be callable from signal handlers, but it is possible to override this behavior
and use malloc for targets where the risk of stack overflow due to unbounded
stack allocations is a more pressing requirement than the corner case of
allowing execl calls from signal handlers.
---
 newlib/README              |  8 ++++++++
 newlib/configure           | 19 +++++++++++++++++++
 newlib/configure.ac        | 13 +++++++++++++
 newlib/libc/posix/execl.c  | 36 ++++++++++++++++++++++++++++++++++--
 newlib/libc/posix/execle.c | 36 ++++++++++++++++++++++++++++++++++--
 newlib/libc/posix/execlp.c | 31 +++++++++++++++++++++++++++++--
 newlib/newlib.hin          |  3 +++
 7 files changed, 140 insertions(+), 6 deletions(-)

diff --git a/newlib/README b/newlib/README
index 69496f695..e652c4a92 100644
--- a/newlib/README
+++ b/newlib/README
@@ -364,6 +364,14 @@ One feature can be enabled by specifying `--enable-FEATURE=yes' or
      less conversion accuracy.
      Enabled by default.
 
+`--enable-newlib-use-malloc-in-execl'
+     Use malloc instead of alloca in execl, execle and execlp. Using malloc
+     avoids unbounded stack allocations in those functions, preventing stack
+     overflows especially for platforms without virtual memory and small stacks.
+     However, POSIX.1-2008 requires execl and execle to be callable from signal
+     handlers while malloc isn't.
+     Disabled by default.
+
 `--enable-multilib'
      Build many library versions.
      Enabled by default.
diff --git a/newlib/configure b/newlib/configure
index bf8d08100..bd5f641b4 100755
--- a/newlib/configure
+++ b/newlib/configure
@@ -999,6 +999,7 @@ enable_newlib_nano_formatted_io
 enable_newlib_retargetable_locking
 enable_newlib_long_time_t
 enable_newlib_use_gdtoa
+enable_newlib_use_malloc_in_execl
 enable_multilib
 enable_target_optspace
 enable_malloc_debugging
@@ -1668,6 +1669,7 @@ Optional Features:
   --enable-newlib-retargetable-locking    Allow locking routines to be retargeted at link time
   --enable-newlib-long-time_t   define time_t to long
   --enable-newlib-use-gdtoa   Use gdtoa rather than legacy ldtoa
+  --enable-newlib-use-malloc-in-execl   use malloc instead of alloca in execl, execle and execlp
   --enable-multilib       build many library versions (default)
   --enable-target-optspace  optimize for space
   --enable-malloc-debugging indicate malloc debugging requested
@@ -2594,6 +2596,17 @@ else
   newlib_use_gdtoa=yes
 fi
 
+# Check whether --enable-newlib-use-malloc-in-execl was given.
+if test "${enable_newlib_use_malloc_in_execl+set}" = set; then :
+  enableval=$enable_newlib_use_malloc_in_execl; case "${enableval}" in
+  yes) newlib_use_malloc_in_execl=yes;;
+  no)  newlib_use_malloc_in_execl=no ;;
+  *)   as_fn_error $? "bad value ${enableval} for newlib-use-malloc-in-execl option" "$LINENO" 5 ;;
+ esac
+else
+  newlib_use_malloc_in_execl=no
+fi
+
 # Default to --enable-multilib
 # Check whether --enable-multilib was given.
 if test "${enable_multilib+set}" = set; then :
@@ -6674,6 +6687,12 @@ $as_echo "#define _WANT_USE_GDTOA 1" >>confdefs.h
 
 fi
 
+if test "${newlib_use_malloc_in_execl}" = "yes"; then
+
+$as_echo "#define _EXECL_USE_MALLOC 1" >>confdefs.h
+
+fi
+
 
 if test "x${iconv_encodings}" != "x" \
    || test "x${iconv_to_encodings}" != "x" \
diff --git a/newlib/configure.ac b/newlib/configure.ac
index 55e5a9446..c6833cfb1 100644
--- a/newlib/configure.ac
+++ b/newlib/configure.ac
@@ -313,6 +313,15 @@ AC_ARG_ENABLE(newlib-use-gdtoa,
   esac
  fi], [newlib_use_gdtoa=yes])dnl
 
+dnl Support --enable-newlib-use-malloc-in-execl
+AC_ARG_ENABLE(newlib-use-malloc-in-execl,
+[  --enable-newlib-use-malloc-in-execl   use malloc instead of alloca in execl, execle and execlp],
+[case "${enableval}" in
+  yes) newlib_use_malloc_in_execl=yes;;
+  no)  newlib_use_malloc_in_execl=no ;;
+  *)   AC_MSG_ERROR(bad value ${enableval} for newlib-use-malloc-in-execl option) ;;
+ esac], [newlib_use_malloc_in_execl=no])dnl
+
 AM_ENABLE_MULTILIB(, ..)
 NEWLIB_CONFIGURE(.)
 
@@ -527,6 +536,10 @@ if test "${newlib_use_gdtoa}" = "yes"; then
   AC_DEFINE(_WANT_USE_GDTOA, 1, [Define if using gdtoa rather than legacy ldtoa.])
 fi
 
+if test "${newlib_use_malloc_in_execl}" = "yes"; then
+  AC_DEFINE(_EXECL_USE_MALLOC, 1, [Define if using malloc for execl, execle and execlp.])
+fi
+
 dnl
 dnl Parse --enable-newlib-iconv-encodings option argument
 dnl
diff --git a/newlib/libc/posix/execl.c b/newlib/libc/posix/execl.c
index c3b4e55bd..9396f67e2 100644
--- a/newlib/libc/posix/execl.c
+++ b/newlib/libc/posix/execl.c
@@ -7,6 +7,12 @@
 
 #include <_ansi.h>
 #include <unistd.h>
+#ifdef _EXECL_USE_MALLOC
+#include <errno.h>
+#include <stdlib.h>
+#else
+#include <alloca.h>
+#endif
 
 /* Only deal with a pointer to environ, to work around subtle bugs with shared
    libraries and/or small data systems where the user declares his own
@@ -24,7 +30,29 @@ execl (const char *path,
 {
   int i;
   va_list args;
-  const char *argv[256];
+  const char **argv;
+
+  i = 1;
+  va_start (args, arg0);
+  do
+    i++;
+  while (va_arg (args, const char *) != NULL);
+  va_end (args);
+  /* POSIX.1-2008 requires execl() to be callable from signal handlers while
+     malloc() isn't, so we default to using alloca(). However, unbounded stack
+     allocations is at a high risk of stack overflow and undefined behavior in
+     environments without virtual memory and small stacks, so we let targets
+     override the default and use malloc() */
+#ifndef _EXECL_USE_MALLOC
+  argv = alloca (i * sizeof(const char *));
+#else
+  argv = malloc (i * sizeof(const char *));
+  if (argv == NULL)
+  {
+    errno = ENOMEM;
+    return -1;
+  }
+#endif
 
   va_start (args, arg0);
   argv[0] = arg0;
@@ -34,6 +62,10 @@ execl (const char *path,
   while (argv[i++] != NULL);
   va_end (args);
 
-  return _execve (path, (char * const  *) argv, *p_environ);
+  i = _execve (path, (char * const  *) argv, *p_environ);
+#ifdef _EXECL_USE_MALLOC
+  free (argv);
+#endif
+  return i;
 }
 #endif /* !_NO_EXECVE  */
diff --git a/newlib/libc/posix/execle.c b/newlib/libc/posix/execle.c
index 34f0ea373..4ef95e497 100644
--- a/newlib/libc/posix/execle.c
+++ b/newlib/libc/posix/execle.c
@@ -7,6 +7,12 @@
 
 #include <_ansi.h>
 #include <unistd.h>
+#ifdef _EXECL_USE_MALLOC
+#include <errno.h>
+#include <stdlib.h>
+#else
+#include <alloca.h>
+#endif
 
 
 #include <stdarg.h>
@@ -20,7 +26,29 @@ execle (const char *path,
   int i;
   va_list args;
   const char * const *envp;
-  const char *argv[256];
+  const char **argv;
+
+  i = 1;
+  va_start (args, arg0);
+  do
+    i++;
+  while (va_arg (args, const char *) != NULL);
+  va_end (args);
+  /* POSIX.1-2008 requires execle() to be callable from signal handlers while
+     malloc() isn't, so we default to using alloca(). However, unbounded stack
+     allocations is at a high risk of stack overflow and undefined behavior in
+     environments without virtual memory and small stacks, so we let targets
+     override the default and use malloc() */
+#ifndef _EXECL_USE_MALLOC
+  argv = alloca (i * sizeof(const char *));
+#else
+  argv = malloc (i * sizeof(const char *));
+  if (argv == NULL)
+  {
+    errno = ENOMEM;
+    return -1;
+  }
+#endif
 
   va_start (args, arg0);
   argv[0] = arg0;
@@ -31,7 +59,11 @@ execle (const char *path,
   envp = va_arg (args, const char * const *);
   va_end (args);
 
-  return _execve (path, (char * const *) argv, (char * const *) envp);
+  i = _execve (path, (char * const *) argv, (char * const *) envp);
+#ifdef _EXECL_USE_MALLOC
+  free (argv);
+#endif
+  return i;
 }
 
 #endif /* !_NO_EXECVE  */
diff --git a/newlib/libc/posix/execlp.c b/newlib/libc/posix/execlp.c
index b845c88c5..4b7959fc8 100644
--- a/newlib/libc/posix/execlp.c
+++ b/newlib/libc/posix/execlp.c
@@ -7,6 +7,12 @@
 
 #include <_ansi.h>
 #include <unistd.h>
+#ifdef _EXECL_USE_MALLOC
+#include <errno.h>
+#include <stdlib.h>
+#else
+#include <alloca.h>
+#endif
 
 
 #include <stdarg.h>
@@ -19,7 +25,24 @@ execlp (const char *path,
 {
   int i;
   va_list args;
-  const char *argv[256];
+  const char **argv;
+
+  i = 1;
+  va_start (args, arg0);
+  do
+    i++;
+  while (va_arg (args, const char *) != NULL);
+  va_end (args);
+#ifndef _EXECL_USE_MALLOC
+  argv = alloca (i * sizeof(const char *));
+#else
+  argv = malloc (i * sizeof(const char *));
+  if (argv == NULL)
+  {
+    errno = ENOMEM;
+    return -1;
+  }
+#endif
 
   va_start (args, arg0);
   argv[0] = arg0;
@@ -29,7 +52,11 @@ execlp (const char *path,
   while (argv[i++] != NULL);
   va_end (args);
 
-  return execvp (path, (char * const *) argv);
+  i = execvp (path, (char * const *) argv);
+#ifdef _EXECL_USE_MALLOC
+  free (argv);
+#endif
+  return i;
 }
 
 #endif /* !_NO_EXECVE  */
diff --git a/newlib/newlib.hin b/newlib/newlib.hin
index 831846940..22e323e48 100644
--- a/newlib/newlib.hin
+++ b/newlib/newlib.hin
@@ -35,6 +35,9 @@
 /* EL/IX level */
 #undef _ELIX_LEVEL
 
+/* Define if using malloc for execl, execle and execlp. */
+#undef _EXECL_USE_MALLOC
+
 /* Define if fseek functions support seek optimization. */
 #undef _FSEEK_OPTIMIZATION
 
-- 
2.39.5