From 09c70dec08529a2a9c552681c8e7c5c1c3f3a6ae Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Sat, 10 Mar 2007 22:51:20 +0000 Subject: [PATCH] Eygene Ryabinkin fixed a use-after-free issue with HTTP transfers with the multi interface --- CHANGES | 20 ++++++++++++++++++++ RELEASE-NOTES | 3 ++- lib/url.c | 4 ++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 129dbdc34..7c473de95 100644 --- a/CHANGES +++ b/CHANGES @@ -7,6 +7,26 @@ Changelog Daniel (10 March 2007) +- Eygene Ryabinkin: + + The problem is the following: when we're calling Curl_done and it decides to + keep the connection opened ('left intact'), then the caller is not notified + that the connection was done via the NULLifying of the pointer, so some easy + handle is keeping the pointer to this connection. + + Later ConnectionExists can select such connection for reuse even if we're + not pipelining: pipeLen is zero, so the (pipeLen > 0 && !canPipeline) is + false and we can reuse this connection for another easy handle. But thus the + connection will be shared between two easy handles if the handle that wants + to take the ownership is not the same as was not notified of the connection + was done in Curl_done. And when some of these easy handles will get their + connection really freed the another one will still keep the pointer. + + My fix was rather trivial: I just added the NULLification to the 'else' + branch in the Curl_done. My tests with Git and ElectricFence showed no + problems both for HTTP pulling and cloning. Repository size is about 250 Mb, + so it was a considerable amount of Curl's work. + - Bryan Henderson introduces two things: 1) the progress callback gets called more frequently (at times) 2) libcurl *might* call the callback when it receives a signal: diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 469a18cde..2d5030026 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -41,6 +41,7 @@ This release includes the following bugfixes: o CURLOPT_INTERFACE for ipv6 o the progress callback can get called more frequently o libcurl might call the progress callback when it receives a signal + o use-after-free issue with HTTP transfers with the multi interface This release includes the following known bugs: @@ -62,6 +63,6 @@ advice from friends like these: Rob Crittenden, Robert A. Monat, Dan Fandrich, Duncan Mac-Vicar Prett, Michal Marek, Robson Braga Araujo, Ian Turner, Linus Nielsen Feltzing, Ravi Pratap, Adam D. Moss, Jose Kahan, Hang Kin Lau, Justin Fletcher, - Robert Iakobashvili, Bryan Henderson + Robert Iakobashvili, Bryan Henderson, Eygene Ryabinkin Thanks! (and sorry if I forgot to mention someone) diff --git a/lib/url.c b/lib/url.c index b80d79239..644cec39a 100644 --- a/lib/url.c +++ b/lib/url.c @@ -4240,6 +4240,10 @@ CURLcode Curl_done(struct connectdata **connp, infof(data, "Connection #%ld to host %s left intact\n", conn->connectindex, conn->bits.httpproxy?conn->proxy.dispname:conn->host.dispname); + + *connp = NULL; /* to make the caller of this function better detect that + this connection is handed over and no longer used from + this point on */ } return result;