Fabian Mauchle <fabian.mauchle(a)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(a)nordu.net, creating an issue in
the bug tracker. Very useful.