Re: [PATCH] setsourcefilter: Use malloc() rather than alloca().

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

On Mon, May 15, 2023 at 03:35:10PM -0300, Adhemerval Zanella Netto wrote:
> 
> 
> On 11/05/23 12:09, Joe Simmons-Talbott via Libc-alpha wrote:
> > To prevent possible stack overflow use malloc() rather than alloca().
> > ---
> >  sysdeps/unix/sysv/linux/setsourcefilter.c | 22 ++++++----------------
> >  1 file changed, 6 insertions(+), 16 deletions(-)
> > 
> > diff --git a/sysdeps/unix/sysv/linux/setsourcefilter.c b/sysdeps/unix/sysv/linux/setsourcefilter.c
> > index 538f4de696..6678572968 100644
> > --- a/sysdeps/unix/sysv/linux/setsourcefilter.c
> > +++ b/sysdeps/unix/sysv/linux/setsourcefilter.c
> > @@ -16,7 +16,6 @@
> >     License along with the GNU C Library; if not, see
> >     <https://www.gnu.org/licenses/>.  */
> >  
> > -#include <alloca.h>
> >  #include <errno.h>
> >  #include <stdlib.h>
> >  #include <string.h>
> > @@ -34,17 +33,11 @@ setsourcefilter (int s, uint32_t interface, const struct sockaddr *group,
> >    /* We have to create an struct ip_msfilter object which we can pass
> >       to the kernel.  */
> >    size_t needed = GROUP_FILTER_SIZE (numsrc);
> > -  int use_alloca = __libc_use_alloca (needed);
> >  
> >    struct group_filter *gf;
> > -  if (use_alloca)
> > -    gf = (struct group_filter *) alloca (needed);
> > -  else
> > -    {
> > -      gf = (struct group_filter *) malloc (needed);
> > -      if (gf == NULL)
> > -	return -1;
> > -    }
> > +  gf = (struct group_filter *) malloc (needed);
> > +  if (gf == NULL)
> > +    return -1;
> >  
> >    gf->gf_interface = interface;
> >    memcpy (&gf->gf_group, group, grouplen);
> > @@ -63,12 +56,9 @@ setsourcefilter (int s, uint32_t interface, const struct sockaddr *group,
> >    else
> >      result = __setsockopt (s, sol, MCAST_MSFILTER, gf, needed);
> >  
> > -  if (! use_alloca)
> > -    {
> > -      int save_errno = errno;
> > -      free (gf);
> > -      __set_errno (save_errno);
> > -    }
> > +  int save_errno = errno;
> > +  free (gf);
> > +  __set_errno (save_errno);
> >  
> >    return result;
> >  }
> 
> free now must preserve errno (check 69fda43b8dd795c3658869633ca0708ed3134006),
> so there is no need to save/restore it.  The rest looks ok for me.
> 

Thanks.  Fixed in v2.

Joe