Re: [PATCH] Improve execl* functions
Federico Terraneo <[email protected]>
| Newsgroups | gmane.comp.lib.newlib |
|---|---|
| Message-ID | <PAXP251MB0317F3C2E06B14A493DD2BD5F9EE2@PAXP251MB0317.EURP251.PROD.OUTLOOK.COM> |
On 27/01/25 18:26, Corinna Vinschen wrote: > But it would still be nice to have a more dynamic solution which doesn't > need ARG_NUM_MAX. Hi Corinna, I spent some time into it, and it's more complicated than I wanted, but I have a patch in this direction nonetheless, that replaces the previous patch proposal. The complexity is caused by POSIX.1-2008 which as far as I could tell requires execl* functions to be callable from signal handlers, while malloc isn't. That leaves alloca as the only way I can think of to lift the argument number limitation. The issue is, newlib targets also microcontroller targets where there's no virtual memory and stacks are small (our Miosix OS is one example), thus unbounded stack allocation (or just large stack allocations like the 1024bytes array in the current version of the execl* code) cause stack overflows and undefined behavior. Thus I propose the attached patch that by default uses alloca but allows individual targets to define _EXECL_USE_MALLOC if in their use case the risk of stack overflows and consequent undefined behavior is a more pressing requirement than the corner case of allowing execl* calls from signal handlers. Even with the default alloca implementation, the patched code uses *less* stack than the current code when execl* functions are called with less that 256 arguments, and on platforms where the stack can grow (i.e, when virtual memory is available) it entirely avoids the stack buffer overflow that was present in the current code, so I think it's an overall improvement. Best regards, Federico Terraneo
0001-Lift-256-arg-limit-in-exec.patch
(text/x-patch, 3.9 KB)
From 223a3958518c5252d575dae9464ec6085a2604b3 Mon Sep 17 00:00:00 2001 From: Terraneo Federico <[email protected]> Date: Tue, 28 Jan 2025 17:19:18 +0100 Subject: [PATCH] Lift 256 arg limit in exec* --- newlib/libc/posix/execl.c | 28 ++++++++++++++++++++++++++-- newlib/libc/posix/execle.c | 28 ++++++++++++++++++++++++++-- newlib/libc/posix/execlp.c | 28 ++++++++++++++++++++++++++-- 3 files changed, 78 insertions(+), 6 deletions(-) diff --git a/newlib/libc/posix/execl.c b/newlib/libc/posix/execl.c index c3b4e55bd..abdf6edb9 100644 --- a/newlib/libc/posix/execl.c +++ b/newlib/libc/posix/execl.c @@ -7,6 +7,9 @@ #include <_ansi.h> #include <unistd.h> +#include <errno.h> +#include <stdlib.h> +#include <alloca.h> /* 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 +27,24 @@ 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); +#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 +54,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..1a944e99a 100644 --- a/newlib/libc/posix/execle.c +++ b/newlib/libc/posix/execle.c @@ -7,6 +7,9 @@ #include <_ansi.h> #include <unistd.h> +#include <errno.h> +#include <stdlib.h> +#include <alloca.h> #include <stdarg.h> @@ -20,7 +23,24 @@ 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); +#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 +51,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..214dc77f8 100644 --- a/newlib/libc/posix/execlp.c +++ b/newlib/libc/posix/execlp.c @@ -7,6 +7,9 @@ #include <_ansi.h> #include <unistd.h> +#include <errno.h> +#include <stdlib.h> +#include <alloca.h> #include <stdarg.h> @@ -19,7 +22,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 +49,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 */ -- 2.39.5