Fabian Mauchle fabian.mauchle@switch.ch wrote Thu, 15 Sep 2016 11:12:56 +0200:
Hi All,
There is a race condition in both the tcp and tls code (in functions tcplistener and tlslistener) when accepting new connections. If new connections arrive in very short succession, and the second (or any further) accept() call returns before the previously created thread has actually started off, these threads operate on the wrong socket. (the pointer handed to the thread points to the local variable s in [tcp|tls]listener() which gets its new value before the thread could copy its value)
This was identified some time ago (thanks Ralf) and https://project.nordu.net/browse/RADSECPROXY-64 was created to track the fix. Branch 'stompless' contains my proposed fix which haven't made it to master (or the maintenance) branch yet. Thanks for rediscovering it and providing a patch!
Proposed change: copy the variable to the heap and use this as argument to the new thread; also new thread is responsible for the memory if spawn is successful.
The argument is an integer. Is there anything wrong with just passing it as is? (Yes, I'm asking for review of branch 'stompless'. ;))
Additional side-fix: tcpreadtimeout() does a select() on a write-fd with the intention to wait for readable bytes. This was fixed in 1.6.3 for tls code, but not tcp.
Good catch! Committed to master and pushed. Thanks!
Also, thanks for CC to radsecproxy-bugs@nordu.net, creating an issue in the bug tracker. Very useful.