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

To prevent possible stack overflow use malloc() rather than alloca().
---
Changes to v1:
  - don't save and restore errno around free()

 sysdeps/unix/sysv/linux/setsourcefilter.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

On 15/05/23 16:08, Joe Simmons-Talbott via Libc-alpha wrote:
> To prevent possible stack overflow use malloc() rather than alloca().
> ---
> Changes to v1:
>   - don't save and restore errno around free()
> 
>  sysdeps/unix/sysv/linux/setsourcefilter.c | 20 ++++----------------
>  1 file changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/setsourcefilter.c b/sysdeps/unix/sysv/linux/setsourcefilter.c
> index 538f4de696..2823168d34 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,7 @@ 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);
> -    }
> +  free (gf);
>  
>    return result;
>  }

Maybe we can use a scratch_buffer to avoid always use malloc:

diff --git a/sysdeps/unix/sysv/linux/setsourcefilter.c b/sysdeps/unix/sysv/linux/setsourcefilter.c
index 538f4de696..22b307ca80 100644
--- a/sysdeps/unix/sysv/linux/setsourcefilter.c
+++ b/sysdeps/unix/sysv/linux/setsourcefilter.c
@@ -16,13 +16,10 @@
    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>
-#include <stdint.h>
 #include <netinet/in.h>
-#include <sys/socket.h>
+#include <scratch_buffer.h>
 #include "getsourcefilter.h"


@@ -34,17 +31,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;
-    }
+  struct scratch_buffer buf;
+  scratch_buffer_init (&buf);
+  if (!scratch_buffer_set_array_size (&buf, 1, needed))
+    return -1;
+  struct group_filter *gf = buf.data;

   gf->gf_interface = interface;
   memcpy (&gf->gf_group, group, grouplen);
@@ -63,12 +54,7 @@ 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);
-    }
+  scratch_buffer_free (&buf);

   return result;
 }

On Tue, May 16, 2023 at 08:09:13AM -0300, Adhemerval Zanella Netto wrote:
> 
> 
> On 15/05/23 16:08, Joe Simmons-Talbott via Libc-alpha wrote:
> > To prevent possible stack overflow use malloc() rather than alloca().
> > ---
> > Changes to v1:
> >   - don't save and restore errno around free()
> > 
> >  sysdeps/unix/sysv/linux/setsourcefilter.c | 20 ++++----------------
> >  1 file changed, 4 insertions(+), 16 deletions(-)
> > 
> > diff --git a/sysdeps/unix/sysv/linux/setsourcefilter.c b/sysdeps/unix/sysv/linux/setsourcefilter.c
> > index 538f4de696..2823168d34 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,7 @@ 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);
> > -    }
> > +  free (gf);
> >  
> >    return result;
> >  }
> 
> Maybe we can use a scratch_buffer to avoid always use malloc:
> 
> diff --git a/sysdeps/unix/sysv/linux/setsourcefilter.c b/sysdeps/unix/sysv/linux/setsourcefilter.c
> index 538f4de696..22b307ca80 100644
> --- a/sysdeps/unix/sysv/linux/setsourcefilter.c
> +++ b/sysdeps/unix/sysv/linux/setsourcefilter.c
> @@ -16,13 +16,10 @@
>     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>
> -#include <stdint.h>
>  #include <netinet/in.h>
> -#include <sys/socket.h>
> +#include <scratch_buffer.h>
>  #include "getsourcefilter.h"
> 
> 
> @@ -34,17 +31,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;
> -    }
> +  struct scratch_buffer buf;
> +  scratch_buffer_init (&buf);
> +  if (!scratch_buffer_set_array_size (&buf, 1, needed))
> +    return -1;
> +  struct group_filter *gf = buf.data;
> 
>    gf->gf_interface = interface;
>    memcpy (&gf->gf_group, group, grouplen);
> @@ -63,12 +54,7 @@ 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);
> -    }
> +  scratch_buffer_free (&buf);
> 
>    return result;
>  }
> 

Thanks for the suggestion.  This patch is now superseded by a new patch
[1]

Thanks,
Joe

[1] https://sourceware.org/pipermail/libc-alpha/2023-May/148146.html