Re: [PATCH] grantpt: Get rid of alloca

[Joe Simmons-Talbott <josimmon@redhat.com>]

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.

Thanks,
Joe
> 
> Maybe the hurd maintainers could review this change?
> 
> Thanks,
> Florian
>