Socket::resolve isn't thread safe

Bug #590359 reported by Razzloss on 2010-06-06
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
DC++
Medium
Razzloss
LinuxDC++
Medium
Razzloss

Bug Description

gethostbyname() shouldn't be called from multiple threads in Linux. As it may return pointer to static data that will be overwritten by the next call. (And based on the few stacktraces I've seen lately it is overwritten.) Windows seems to allocate the data for gethostbyname from thread local storage, so that will probably explain why it took so long to be reported.

Related branches

Razzloss (razzloss) wrote :

Windows people, can you check if this patch can be compiled/used with DC++ or does it need to be #ifdeffed.

--EZ

The text leading up to this was:
--------------------------
|=== modified file 'dcpp/Socket.cpp'
|--- dcpp/Socket.cpp 2010-02-11 21:44:13 +0000
|+++ dcpp/Socket.cpp 2010-06-06 10:00:06 +0000
--------------------------
Patching file dcpp/Socket.cpp using Plan A...
Hunk #1 succeeded at 530.
Hmm... Ignoring the trailing garbage.
done

dcpp\Socket.cpp:533:18: error: aggregate 'dcpp::Socket::resolve(const std::string&)::addrinfo hints'
 has incomplete type and cannot be defined
dcpp\Socket.cpp:535:42: error: invalid application of 'sizeof' to incomplete type 'dcpp::Socket::res
olve(const std::string&)::addrinfo'
dcpp\Socket.cpp:538:53: error: 'getaddrinfo' was not declared in this scope
dcpp\Socket.cpp:539:13: error: invalid use of incomplete type 'struct dcpp::Socket::resolve(const st
d::string&)::addrinfo'
dcpp\Socket.cpp:533:9: error: forward declaration of 'struct dcpp::Socket::resolve(const std::string
&)::addrinfo'
dcpp\Socket.cpp:540:46: error: invalid use of incomplete type 'struct dcpp::Socket::resolve(const st
d::string&)::addrinfo'
dcpp\Socket.cpp:533:9: error: forward declaration of 'struct dcpp::Socket::resolve(const std::string
&)::addrinfo'
dcpp\Socket.cpp:542:22: error: 'freeaddrinfo' was not declared in this scope
scons: *** [build\debug-mingw\dcpp\Socket.o] Error 1
scons: building terminated because of errors.

poy (poy) wrote :

makes sense since gethostbyname is deprecated.
however, i seem to recall that getaddrinfo isn't always available on every system? that being the reason that gethostbyname is still being so widely used. not that it matters to Windows as the function is present here.

could you get rid of some of the C-isms? no need to re-declare "struct"s, and the memset can be changed to just "addrinfo hints = { 0 }".

Razzloss (razzloss) wrote :

ok, I used #ifdef for _WIN32.

--RZ

Changed in dcplusplus:
assignee: nobody → Razzloss (razzloss)
importance: Undecided → Medium
status: New → Fix Committed
Razzloss (razzloss) wrote :

Crap, just committed that.

Razzloss (razzloss) wrote :

OK, fixed the C-isms.

I'd imagine most modern systems should support getaddrinfo by now. (At least Linux and BSD, Solaris might not). But anyway it probably is the only re-entrant safe alternative (gethostbyname2 and gethostname_r being GNU extensions).

--RZ

Changed in linuxdcpp:
status: Confirmed → Fix Committed
poy (poy) wrote :

Fixed in DC++ 0.770.

Changed in dcplusplus:
status: Fix Committed → Fix Released
Changed in linuxdcpp:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers