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: Pengcheng Chen <pengcheng dot chen at gmail dot com>
- To: Ondřej Bílka <neleai at seznam dot cz>
- Cc: "Banerjee, Debabrata" <dbanerje at akamai 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 10:13:16 -0800
- 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> <20140117002246 dot GA14011 at domone dot podge>
- Reply-to: Pengcheng dot Chen at gmail dot com
On Thu, Jan 16, 2014 at 4:22 PM, OndÅej BÃlka <neleai@seznam.cz> wrote:
> 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.
Good suggestion. I totally agree with you. This make code simpler.
>
> Pengcheng could you do this?
Before I take on this task, can you please explain a little bit about
the process? I'm new to glibc and am still sort of struggling to build
it.
Thanks,
Pengcheng