[PATCH] grantpt: Get rid of alloca

[Joe Simmons-Talbott]

On Mon, Jun 05, 2023 at 03:14:06PM -0300, Adhemerval Zanella Netto wrote:
> 
> 
> 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 tested this patch today with build-many-glibcs.py for i686-gnu and it
passed 'make check'.  Does your second sentence mean that I needed to
pass '--enable-pt_chown' to configure for the test build?

Thanks,
Joe