curl-library
Re: Curl 7.16.x and multi interface
Date: Sun, 11 Mar 2007 01:09:18 +0300
Good day!
Finaly, I've tracked the use-after-free issue.
Sat, Mar 03, 2007 at 11:44:19PM +0300, Eygene Ryabinkin wrote:
> > >From reading lib/url.c I am under impression that the 'reuse =
> > >ConnectionExists' stanza in url.c:3698 can select already used connection to
> > >be used again. Seems like it is done without checking if the pipelining is
> > >enabled. Though, I can be wrong.
> >
> > I don't see how that code allows double use of the same connection without
> > pipelining. Perhaps I'm being stupid, but can you point out exactly how it
> > would get an already used connection?
>
> I am sorry, being wrong: missed the very first bits from the
> ConnectionExists that are checking for the pipelining.
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.
I do not know if the patch will do the competely right thing, but it
seems to me that if Curl_done was called, then the connection associated
with handle should be detached from the latter no matter if we're
closing and freeing the connection just now or leaving it in place
for latter reusal. Though, I can be wrong.
The patch is attached. It was done for the 7.6.12-20070301.
-- Eygene
- text/plain attachment: Curl_done.patch