From 2a94293efd3896eca067c4b53ccfdf362a042db2 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 23 Mar 2010 13:18:30 +0100 Subject: [PATCH] delayed easy handle kill caused double Curl_close() call Hauke Duden provided an example program that made the multi interface crash. His example simply used the multi interface and did first one FTP transfer and after completion it used a second easy handle and did another FTP transfer on the same FTP server. This triggered a bug in the "delayed easy handle kill" system that curl uses: when an FTP connection is left alive it must keep an easy handle around internally - only for the purpose of having an easy handle when it later disconnects it. The code assumed that when the easy handle was removed and an internal reference was made, that version could be killed later on when a new easy handle came using the same connection. This was wrong as Hauke's example showed that the removed handle wasn't killed for real until later. This caused a double close attempt => segfault. --- CHANGES | 15 +++++++++++++++ RELEASE-NOTES | 3 ++- lib/multi.c | 10 ++++++++-- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/CHANGES b/CHANGES index 8d812727f..174fbc69f 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,21 @@ Changelog +Daniel Stenberg (23 Mar 2010) +- Hauke Duden provided an example program that made the multi interface crash. + His example simply used the multi interface and did first one FTP transfer + and after completion it used a second easy handle and did another FTP + transfer on the same FTP server. + + This triggered a bug in the "delayed easy handle kill" system that curl + uses: when an FTP connection is left alive it must keep an easy handle + around internally - only for the purpose of having an easy handle when it + later disconnects it. The code assumed that when the easy handle was removed + and an internal reference was made, that version could be killed later on + when a new easy handle came using the same connection. This was wrong as + Hauke's example showed that the removed handle wasn't killed for real until + later. This caused a double close attempt => segfault. + Daniel Stenberg (22 Mar 2010) - Thomas Lopatic fixed the alarm()-based DNS timeout: diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 072ad7e6b..6d0b7963d 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -36,6 +36,7 @@ This release includes the following bugfixes: o TFTP fixed TSIZE handling for uploads o SSL possible double free when reusing curl handle o alarm()-based DNS timeout bug + o re-used FTP connection multi interface crash This release includes the following known bugs: @@ -47,6 +48,6 @@ advice from friends like these: Steven M. Schweda, Yang Tse, Jack Zhang, Tom Donovan, Martin Hager, Daniel Fandrich, Patrick Monnerat, Pat Ray, Wesley Miaw, Ben Greear, Ryan Chan, Markus Duft, Andrei Benea, Jacob Moshenko, Daniel Johnson, - Constantine Sapuntzakis, Douglas Steinwand, Thomas Lopatic + Constantine Sapuntzakis, Douglas Steinwand, Thomas Lopatic, Hauke Duden Thanks! (and sorry if I forgot to mention someone) diff --git a/lib/multi.c b/lib/multi.c index b24021280..d04fcf660 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -2416,10 +2416,16 @@ static CURLMcode add_closure(struct Curl_multi *multi, if(!inuse) { /* cl->easy_handle is now killable */ - infof(data, "Delayed kill of easy handle %p\n", cl->easy_handle); + /* unmark it as not having a connection around that uses it anymore */ cl->easy_handle->state.shared_conn= NULL; - Curl_close(cl->easy_handle); + + if(cl->easy_handle->state.closed) { + infof(data, "Delayed kill of easy handle %p\n", cl->easy_handle); + /* close handle only if curl_easy_cleanup() already has been called + for this easy handle */ + Curl_close(cl->easy_handle); + } if(p) p->next = n; else