This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2 1/3] Fix __check_pf()/make_request() stack overflow segfault (convert to malloc)
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: "Banerjee, Debabrata" <dbanerje at akamai dot com>
- Cc: "Pengcheng dot Chen at gmail dot com" <Pengcheng dot Chen at gmail dot com>, "libc-alpha at sourceware dot org" <libc-alpha at sourceware dot org>, Carlos O'Donell <carlos at redhat dot com>
- Date: Fri, 17 Jan 2014 01:22:46 +0100
- Subject: Re: [PATCH v2 1/3] Fix __check_pf()/make_request() stack overflow segfault (convert to malloc)
- Authentication-results: sourceware.org; auth=none
- References: <524E4504 dot 6050603 at redhat dot com> <1383268213-14349-1-git-send-email-dbanerje at akamai dot com> <20140116225341 dot GA23189 at domone dot podge> <CEFDCD71 dot 2C670%dbanerje at akamai dot com>
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?