This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Don't bind to registered ports in bindresvport
On 6/5/2012 8:46 PM, Petr Baudis wrote:
> On Fri, Jun 01, 2012 at 11:03:53PM -0400, Carlos O'Donell wrote:
>> On Fri, Jun 1, 2012 at 4:01 PM, Dan Nicholson <dbn.lists@gmail.com> wrote:
>>> @@ -74,24 +76,50 @@ bindresvport (int sd, struct sockaddr_in *sin)
>>>
>>> int nports = ENDPORT - startport + 1;
>>> int endport = ENDPORT;
>>> + bool_t chkport = TRUE;
>>> + struct servent serv;
>>> + char buf[1024];
>>
>> A global static buffer of this size should come from malloc (and get
>> allocated once).
>
> Is there a reason for explicitly making this non-thread-safe? Won't
> that break existing programs?
>
> (The current code also uses two statics, but running the function
> multiple times in parallel shouldn't result in anything worse than
> slightly scattered port allocations, if I read the code right.)
This function is not listed as being thread-safe or async-signal safe
in any API that I've seen documented.
I do not think the code is thread-safe, for example:
80 sin->sin_port = htons (port++);
81 if (port > endport)
82 port = startport;
83 res = __bind (sd, sin, sizeof (struct sockaddr_in));
84 if (res >= 0 || errno != EADDRINUSE)
85 break;
If N threads all execute port++ on line 80 the values for sin_port
may exceed endport, they are not adjusted, __bind succeeds
with the non-privileged port allocation, and you've violated the API.
>> There is precedence for using alloca, but I'd like to avoid that.
>
> Why? __MAX_ALLOCA_CUTOFF is currently at 65536, so 1024 bytes on
> stack should be a non-issue.
Using alloca can create a security risk by putting the buffer in
in the same location every time relative to the stack pointer or
call sequence. While with malloc the allocation locations can be
random.
Cheers,
Carlos.
--
Carlos O'Donell
Mentor Graphics / CodeSourcery
carlos_odonell@mentor.com
carlos@codesourcery.com
+1 (613) 963 1026