Account for alloca use when collecting interface addresses

# ip li add name dummy0 type dummy
# site_id=$(head -c6 /dev/urandom | od -tx2 -An | tr ' ' ':')
# for ((i = 0; i < 65536; i++)) do
> ip ad ad $(printf fd80$site_id::%04x $i)/128 dev dummy0
> done
# (ulimit -s 900; getent ahosts localhost)
# ip li de dummy0

	[BZ #16002]
	* sysdeps/unix/sysv/linux/check_pf.c (make_request): Use
	alloca_account and account alloca use for struct in6ailist.
---
 sysdeps/unix/sysv/linux/check_pf.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

On Mon, Mar 24, 2014 at 03:04:23PM +0100, Andreas Schwab wrote:
> # ip li add name dummy0 type dummy
> # site_id=$(head -c6 /dev/urandom | od -tx2 -An | tr ' ' ':')
> # for ((i = 0; i < 65536; i++)) do
> > ip ad ad $(printf fd80$site_id::%04x $i)/128 dev dummy0
> > done
> # (ulimit -s 900; getent ahosts localhost)
> # ip li de dummy0

This needs a more verbose description for the commit log.  The change
itself looks OK to me.

Thanks,
Siddhesh

> 	[BZ #16002]
> 	* sysdeps/unix/sysv/linux/check_pf.c (make_request): Use
> 	alloca_account and account alloca use for struct in6ailist.
> ---
>  sysdeps/unix/sysv/linux/check_pf.c | 32 ++++++++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/check_pf.c b/sysdeps/unix/sysv/linux/check_pf.c
> index 5c8e75a..6d8468d 100644
> --- a/sysdeps/unix/sysv/linux/check_pf.c
> +++ b/sysdeps/unix/sysv/linux/check_pf.c
> @@ -139,9 +139,10 @@ make_request (int fd, pid_t pid)
>  #endif
>    bool use_malloc = false;
>    char *buf;
> +  size_t alloca_used = 0;
>  
>    if (__libc_use_alloca (buf_size))
> -    buf = alloca (buf_size);
> +    buf = alloca_account (buf_size, alloca_used);
>    else
>      {
>        buf = malloc (buf_size);
> @@ -163,6 +164,7 @@ make_request (int fd, pid_t pid)
>    {
>      struct in6addrinfo info;
>      struct in6ailist *next;
> +    bool use_malloc;
>    } *in6ailist = NULL;
>    size_t in6ailistlen = 0;
>    bool seen_ipv4 = false;
> @@ -239,7 +241,19 @@ make_request (int fd, pid_t pid)
>  		    }
>  		}
>  
> -	      struct in6ailist *newp = alloca (sizeof (*newp));
> +	      struct in6ailist *newp;
> +	      if (__libc_use_alloca (alloca_used + sizeof (*newp)))
> +		{
> +		  newp = alloca_account (sizeof (*newp), alloca_used);
> +		  newp->use_malloc = false;
> +		}
> +	      else
> +		{
> +		  newp = malloc (sizeof (*newp));
> +		  if (newp == NULL)
> +		    goto out_fail;
> +		  newp->use_malloc = true;
> +		}
>  	      newp->info.flags = (((ifam->ifa_flags
>  				    & (IFA_F_DEPRECATED
>  				       | IFA_F_OPTIMISTIC))
> @@ -286,7 +300,10 @@ make_request (int fd, pid_t pid)
>        do
>  	{
>  	  result->in6ai[--in6ailistlen] = in6ailist->info;
> -	  in6ailist = in6ailist->next;
> +	  struct in6ailist *next = in6ailist->next;
> +	  if (in6ailist->use_malloc)
> +	    free (in6ailist);
> +	  in6ailist = next;
>  	}
>        while (in6ailist != NULL);
>      }
> @@ -302,7 +319,14 @@ make_request (int fd, pid_t pid)
>      free (buf);
>    return result;
>  
> -out_fail:
> + out_fail:
> +  while (in6ailist != NULL)
> +    {
> +      struct in6ailist *next = in6ailist->next;
> +      if (in6ailist->use_malloc)
> +	free (in6ailist);
> +      in6ailist = next;
> +    }
>    if (use_malloc)
>      free (buf);
>    return NULL;
> -- 
> 1.9.1
> 
> 
> -- 
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."

Siddhesh Poyarekar <siddhesh@redhat.com> writes:

> On Mon, Mar 24, 2014 at 03:04:23PM +0100, Andreas Schwab wrote:
>> # ip li add name dummy0 type dummy
>> # site_id=$(head -c6 /dev/urandom | od -tx2 -An | tr ' ' ':')
>> # for ((i = 0; i < 65536; i++)) do
>> > ip ad ad $(printf fd80$site_id::%04x $i)/128 dev dummy0
>> > done
>> # (ulimit -s 900; getent ahosts localhost)
>> # ip li de dummy0
>
> This needs a more verbose description for the commit log.

It's just the reproducer, you can try yourself without breaking anything
(unless you already have a dummy0 device).

Andreas.

On Mon, Mar 24, 2014 at 03:41:54PM +0100, Andreas Schwab wrote:
> Siddhesh Poyarekar <siddhesh@redhat.com> writes:
> 
> > On Mon, Mar 24, 2014 at 03:04:23PM +0100, Andreas Schwab wrote:
> >> # ip li add name dummy0 type dummy
> >> # site_id=$(head -c6 /dev/urandom | od -tx2 -An | tr ' ' ':')
> >> # for ((i = 0; i < 65536; i++)) do
> >> > ip ad ad $(printf fd80$site_id::%04x $i)/128 dev dummy0
> >> > done
> >> # (ulimit -s 900; getent ahosts localhost)
> >> # ip li de dummy0
> >
> > This needs a more verbose description for the commit log.
> 
> It's just the reproducer, you can try yourself without breaking anything
> (unless you already have a dummy0 device).

Oh I got that; I just thought that it would be a good idea to put it
in the commit log since we can't add it in the testsuite and hence
suggested putting some English text around it to describe it, which is
kind of what you did in your commit :)

Siddhesh

On Mon, Mar 24, 2014 at 03:04:23PM +0100, Andreas Schwab wrote:
> # ip li add name dummy0 type dummy
> # site_id=$(head -c6 /dev/urandom | od -tx2 -An | tr ' ' ':')
> # for ((i = 0; i < 65536; i++)) do
> > ip ad ad $(printf fd80$site_id::%04x $i)/128 dev dummy0
> > done
> # (ulimit -s 900; getent ahosts localhost)
> # ip li de dummy0
> 
> 	[BZ #16002]
> 	* sysdeps/unix/sysv/linux/check_pf.c (make_request): Use
> 	alloca_account and account alloca use for struct in6ailist.

As far as I recall there was a similar patch that did not get in.

After I reviewed that I noticed that you could drop alloca altogether.

Its only used to construct a single linked list and then copy it to
malloced array. Faster way would be to malloc array directly, realloc as
necessary.

Ondřej Bílka <neleai@seznam.cz> writes:

> As far as I recall there was a similar patch that did not get in.
>
> After I reviewed that I noticed that you could drop alloca altogether.
>
> Its only used to construct a single linked list and then copy it to
> malloced array. Faster way would be to malloc array directly, realloc as
> necessary.

Sorry, I missed it when I searched for __check_pf.  Could you rebase
your patch?  This issue is orthogonal to the alloca issue.

Andreas.

I'm seeing compilation warnings (we clearly need to move to a -Werror 
default, I see a recent patch of mine introduced some as well...):

../sysdeps/unix/sysv/linux/check_pf.c:326:20: warning: 'in6ailist' may be used uninitialized in this function [-Wmaybe-uninitialized]

The warning looks right to me: the change introduced uses of in6ailist 
after the out_fail label, while there are jumps to out_fail from before 
in6ailist is declared and initialized to NULL.