Crash triggered by use-after-free of HttpSM

[lzx404243]

Encountered this crash in production running our internal 9.x that seems like a use-after-free of HttpSM:

#0  Mutex_lock (t=0x7fb2bfffb440, m=0x0)
    at ts/iocore/eventsystem/I_Lock.h
#1  Mutex_lock (m=..., m=..., t=0x7fb2bfffb440)
    at ts/iocore/eventsystem/I_Lock.h
#2  MutexLock::MutexLock (t=0x7fb2bfffb440, am=..., this=0x7fb2b4dfd5e0)
    at ts/iocore/eventsystem/I_Lock.h
#3  ConnectingEntry::state_http_server_open (this=0x7fb186b95200, event=105, 
    data=0x7fb2a58a6280)
    at ts/proxy/http/ConnectingEntry.cc
#4  0x0000000000856a04 in Continuation::handleEvent (data=0x7fb2a58a6280, event=105, 
    this=0x7fb186b95200)
    at ts/iocore/eventsystem/I_Continuation.h
#5  Continuation::handleEvent (data=0x7fb2a58a6280, event=105, this=0x7fb186b95200)
    at ts/iocore/eventsystem/I_Continuation.h
#6  write_signal_and_update (event=105, vc=0x7fb2a58a6000)
    at ts/iocore/net/UnixNetVConnection.cc
#7  0x000000000085b151 in UnixNetVConnection::mainEvent (this=0x7fb2a58a6000, 
    event=<optimized out>, e=<optimized out>)
    at ts/iocore/net/UnixNetVConnection.cc
#8  0x000000000084aadb in InactivityCop::check_inactivity (this=0x7fb2a57ca000, 
    event=<optimized out>, e=0x7fb2a5700060)
    at ts/iocore/net/UnixNet.cc
#9  0x000000000089d79c in Continuation::handleEvent (data=0x7fb2a5700060, event=2, 
    this=<optimized out>)
    at ts/iocore/eventsystem/I_Continuation.h
#10 Continuation::handleEvent (data=0x7fb2a5700060, event=2, this=<optimized out>)
    at ts/iocore/eventsystem/I_Continuation.h
#11 EThread::process_event (this=0x7fb2bfffb440, e=0x7fb2a5700060, calling_code=2)
    at ts/iocore/eventsystem/UnixEThread.cc
#12 0x000000000089e154 in EThread::execute_regular (this=this@entry=0x7fb2bfffb440)
    at ts/iocore/eventsystem/UnixEThread.cc
#13 0x000000000089e676 in EThread::execute (this=0x7fb2bfffb440)
    at ts/iocore/eventsystem/UnixEThread.cc
#14 0x000000000089cc12 in spawn_thread_internal (a=0x7fb2c7e58340)
    at ts/iocore/eventsystem/Thread.cc
#15 0x00007fb2c9cfa1ca in start_thread () from /lib64/libpthread.so.0
#16 0x00007fb2c8e32e73 in clone () from /lib64/libc.so.6

[lzx404243]

The crash is observed to be coincided with changes in #10157, where the port is also considered when comparing entries in ConnectingIpPool.

[lzx404243]

In the core, I noticed that HttpSM of a request from 1.1.1.1:80 was being added to ConnectingEntry of 1.1.1.1:443

I believe this is how the crash happened:

• Suppose both 1.1.1.1:80 and 1.1.1.1:443 are both in the thread Connection map.
• Now, a new request came in from 1.1.1.1:80, let’s call it request A.
• In add_to_existing_request(), find() returns the iterator pointing to the ConnectingEntry for 1.1.1.1:80.
• The following condition is false, so the iterator is incremented: (proposed_hostname == ip_iter->second->hostname && proposed_sni == ip_iter->second->sni && proposed_cert == ip_iter->second->cert_name && ip_iter->second->connect_sms.size() < 50)
• The iterator now points to 1.1.1.1:443, since
  • the hash code is generated based on the IP, therefore the entries for both keys/ips are in the same bucket in the map.
  • as we pointed out, the ip_iter->first == ip condition in the while() loop only checks IP address, not the port . So it is able to continue the iteration on the same ip with different port.
• HttpSM for request from 1.1.1.1:80 is added to a ConnectingEntry of 1.1.1.1:443.
• All HttpSMs are successfully processed for ConnectingEntry for 1.1.1.1:80, the ConnectingEntry for 1.1.1.1:80 is removed from the thread connection map.
• Client abort detected on request A. Tearing down HttpSM
• kill_this() is called. In cancel_pending_server_connection(), it searches the thread connection map with key 1.1.1.1:80. Now since it has been removed from the map, the search would fail. Therefore, the HttpSM, still in ConnectingEntry of 1.1.1.1:443 is not cleaned up.
• When processing all HttpSMs from ConnectingEntry of 1.1.1.1:443, the crash is triggered.

[lzx404243]

So it seems the problem is that the looping condition assumes entries in the ConnectingIpPool is keyed in the IP address only. With #10157, this is no longer true since now it's keyed in both the IP address and port.