Re: [PATCH v2 1/3] Fix __check_pf()/make_request() stack overflow segfault (convert to malloc)

[OndÅej BÃlka <neleai at seznam dot cz>]

On Thu, Jan 16, 2014 at 05:58:31PM -0500, Banerjee, Debabrata wrote:
> On 1/16/14 5:53 PM, "OndÅej BÃlka" <neleai@seznam.cz> wrote:
> 
> >On Thu, Oct 31, 2013 at 09:10:11PM -0400, Debabrata Banerjee wrote:
> >> Tested calls to getaddrinfo() with 64k+ local IPv4 and IPv6 addresses.
> >> 
> >> Changelog:
> >> 
> >> 2013-10-31  Debabrata Banerjee  <dbanerje@akamai.com>
> >> 
> >
> >As that patch was quite large and there is not much time if we want this
> >in 2.19 we should use a simpler patch. I wrote new patch that uses
> >malloc instead.
> 
> Sorry I haven't had time to look into refactoring this patch more. However
> Pengcheng has volunteered to clean it up in whatever way is acceptable.
> 
> The caveat with using malloc is that I would expect it to be significantly
> slower than alloca, but someone can benchmark that. I believe this is a
> hot path.
> 
That is tricky question, bottleneck could also be in network
latency, probably here

 if (TEMP_FAILURE_RETRY (__sendto (fd, (void *) &req, sizeof (req), 0,
                                    (struct sockaddr *) &nladdr,
                                    sizeof (nladdr))) < 0)

Besides of that as I studied a code more carefully I probably found that
these allocations are useless.

We use these entries to store data in linked list, relevant part is

       struct in6ailist *newp = alloca (sizeof (*newp));
       <snip>
       newp->next = in6ailist;
       in6ailist = newp;
       ++in6ailistlen;

t we use this list only in following fragment:

     result = malloc (sizeof (*result)
                       + in6ailistlen * sizeof (struct in6addrinfo));
      <snip>
      do
        {
          result->in6ai[--in6ailistlen] = in6ailist->info;
          in6ailist = in6ailist->next;
        }
      while (in6ailist != NULL);

which only copies list into malloced array while preserving ordering.

A better way would be to malloc result at start and write into in6ai
array directly, calling realloc to double size as necessary. At end we
could optionally trim memory at end.

This should also be more effective as one big copy is faster when you
need to copy same amount of data in small chunks.

Pengcheng could you do this?