Re: [PATCH] grantpt: Get rid of alloca

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 05/06/23 10:37, Joe Simmons-Talbott via Libc-alpha wrote:
> On Mon, Jun 05, 2023 at 03:29:45PM +0200, Florian Weimer wrote:
>> * Joe Simmons-Talbott via Libc-alpha:
>>
>>> Replace alloca with a scratch_buffer to avoid potential stack overflows.
>>> ---
>>>  sysdeps/unix/grantpt.c | 12 +++++++++++-
>>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/sysdeps/unix/grantpt.c b/sysdeps/unix/grantpt.c
>>> index 38fce52576..e5d2390bf2 100644
>>> --- a/sysdeps/unix/grantpt.c
>>> +++ b/sysdeps/unix/grantpt.c
>>> @@ -20,6 +20,7 @@
>>>  #include <fcntl.h>
>>>  #include <grp.h>
>>>  #include <limits.h>
>>> +#include <scratch_buffer.h>
>>>  #include <stdlib.h>
>>>  #include <string.h>
>>>  #include <sys/resource.h>
>>> @@ -147,7 +148,14 @@ grantpt (int fd)
>>>  	/* `sysconf' does not support _SC_GETGR_R_SIZE_MAX.
>>>  	   Try a moderate value.  */
>>>  	grbuflen = 1024;
>>> -      grtmpbuf = (char *) __alloca (grbuflen);
>>> +      struct scratch_buffer sbuf;
>>> +      scratch_buffer_init (&sbuf);
>>> +      if (!scratch_buffer_set_array_size (&sbuf, 1, grbuflen))
>>> +	{
>>> +	  retval -1;
>>> +	  goto cleanup;
>>> +	}
>>> +      grtmpbuf = sbuf.data;
>>>        __getgrnam_r (TTY_GROUP, &grbuf, grtmpbuf, grbuflen, &p);
>>>        if (p != NULL)
>>>  	tty_gid = p->gr_gid;
>>> @@ -255,6 +263,8 @@ grantpt (int fd)
>>>    if (buf != _buf)
>>>      free (buf);
>>>  
>>> +  scratch_buffer_free(sbuf);
>>> +
>>>    return retval;
>>>  }
>>>  libc_hidden_def (grantpt)
>>
>> How did you test this?  This code is only used on Hurd due to the
>> override in sysdeps/unix/sysv/linux/grantpt.c.
> 
> The only testing I did was a 'make && make check' on x86_64 and i686 linux.

You will need to build for i686-gnu (Hurd) to actually enable this code.  Which
leads to another, which Siddheash has brough, which is --enable-pt_chown does
make sense to be provided as a configure switch.

I think we should just move all pt_chown to be Hurd specific, and remove the
--enable-pt_chown option.