Re: [PATCH] Improve execl* functions

On Jan 27 15:36, Federico Terraneo wrote:
> Hi Corinna,
> 
> On 27/01/25 14:21, Corinna Vinschen wrote:
> > Adding ARG_NUM_MAX this way requires all targets to define their own
> > ARG_NUM_MAX if they want to support more than 256 args.  Right now,
> > there's no such limitation.
> 
> As far as I can see, right now there is this limitation in newlib.
> Let's have a look at the current code of one of these functions:
> 
> https://sourceware.org/git?p=newlib-cygwin.git;a=blob_plain;f=newlib/libc/posix/execl.c
> 
> What I see is a
> 
> const char *argv[256];
> 
> declared on the stack, and a do..while loop copying args into that array
> with no bound checking. Thus, attempting to pass more than 256 arguments
> will silently corrupt the stack.
> 
> Attached please find a test case that when compiled with the address
> sanitizer fails due to the stack buffer overflow when more than 256
> arguments are passed to the execl code above.
> 
> The purpose of my patch is to:
> - avoid the stack buffer overflow in all cases
> - allow targets to override the default 256 arguments limit if they so
> desire, either lowering to reduce stack use or increasing it if 256 is not
> enough

Huh, you're right.  Apologies, I was not reading all of your patch and
was thinking of the Cygwin exec(3), which doesn't have that restriction.

I checked the history, the exec functions are using this static stack
allocation since the last century, before the CVS repo was created.

If we keep your patch, please redefine ARG_NUM_MAX to _ARG_NUM_MAX.

But it would still be nice to have a more dynamic solution which doesn't
need ARG_NUM_MAX.


Thanks,
Corinna