------------------------------------------------------------ revno: 13238 revision-id: squid3@treenet.co.nz-20160506094614-zyjq1i8ym2gcvwbu parent: squid3@treenet.co.nz-20160506091244-3td2be9qbzsajxg8 committer: Amos Jeffries branch nick: 3.4 timestamp: Fri 2016-05-06 21:46:14 +1200 message: Fix SIGSEGV in ESIContext response handling HttpReply pointer was being unlocked without heving been locked. Resulting in a double-free. Make it use RefCount instead of manual locking to ensure locked/unlock is always symmetrical. ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20160506094614-zyjq1i8ym2gcvwbu # target_branch: http://bzr.squid-cache.org/bzr/squid3/3.4 # testament_sha1: 4ff01cd3aaffb8c9b20b8595d2ee0ccfcf434b3f # timestamp: 2016-05-06 09:48:45 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/3.4 # base_revision_id: squid3@treenet.co.nz-20160506091244-\ # 3td2be9qbzsajxg8 # # Begin patch === modified file 'src/esi/Esi.cc' --- src/esi/Esi.cc 2016-04-20 11:15:14 +0000 +++ src/esi/Esi.cc 2016-05-06 09:46:14 +0000 @@ -598,7 +598,7 @@ #endif - if (!(rep || (outbound.getRaw() && + if (!(rep != NULL || (outbound.getRaw() && outbound->len && (outbound_offset <= outbound->len)))) { debugs(86, 5, "ESIContext::send: Nothing to send."); return 0; @@ -643,18 +643,18 @@ flags.clientwantsdata = 0; debugs(86, 5, "ESIContext::send: this=" << this << " Client no longer wants data "); /* Deal with re-entrancy */ - HttpReply *temprep = rep; + HttpReply::Pointer temprep = rep; rep = NULL; /* freed downstream */ - if (temprep && varState) - varState->buildVary (temprep); + if (temprep != NULL && varState) + varState->buildVary(temprep.raw()); { StoreIOBuffer tempBuffer; tempBuffer.length = len; tempBuffer.offset = pos - len; tempBuffer.data = next->readBuffer.data; - clientStreamCallback (thisNode, http, temprep, tempBuffer); + clientStreamCallback (thisNode, http, temprep.raw(), tempBuffer); } if (len == 0) @@ -1284,7 +1284,7 @@ ++parserState.stackdepth; } - if (rep && !parserState.inited()) + if (rep != NULL && !parserState.inited()) parserState.init(this); /* we have data */ @@ -1423,7 +1423,7 @@ { debugs(86, 5, HERE << "Freeing for this=" << this); - HTTPMSGUNLOCK(rep); + rep = NULL; // refcounted finishChildren (); diff --git src/esi/Context.h src/esi/Context.h index baae3fd..dcdcf05 100644 --- src/esi/Context.h +++ src/esi/Context.h @@ -36,6 +36,7 @@ #include "esi/Element.h" #include "clientStream.h" #include "err_type.h" +#include "HttpReply.h" #include "HttpStatusCode.h" class ESIVarState; @@ -116,7 +117,7 @@ public: err_type errorpage; /* if we error what page to use */ http_status errorstatus; /* if we error, what code to return */ char *errormessage; /* error to pass to error page */ - HttpReply *rep; /* buffered until we pass data downstream */ + HttpReply::Pointer rep; /* buffered until we pass data downstream */ ESISegment::Pointer buffered; /* unprocessed data - for whatever reason */ ESISegment::Pointer incoming; /* processed data we are waiting to send, or for