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: "Banerjee, Debabrata" <dbanerje at akamai dot com>
- To: Ondřej Bílka <neleai at seznam dot cz>
- 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 16:39:18 -0500
- 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>
On 1/16/14, 7:22 PM, "Ondřej Bílka" <neleai@seznam.cz> wrote:
>On Thu, Jan 16, 2014 at 05:58:31PM -0500, Banerjee, Debabrata wrote:
>>
>>
>> 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)
That's a netlink connection to the kernel, no point in worrying about
latency here.
>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.
Less copies are good but multiple mallocs are bad. What I wonder (and have
spent no time looking at) is if getting all this data is necessary.
Clearly the end result only uses a small fraction of this data. I wonder
if any of this is necessary. My only goal with my patch was to fix the
implementation of what was there, assuming it is necessary.
-Deb