Bugs item #2284386, was opened at 2008-11-14 15:55
Message generated for change (Comment added) made by bagder
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2284386&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: libcurl
Group: wrong behaviour
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Christian Krause (chkr)
Assigned to: Daniel Stenberg (bagder)
Summary: memory leak in lib/http.c

Initial Comment:
Hi,

when using libcurl with GSS/kerberos authentication I'll see a memory leek in lib/http.c:

Here is the debug output of the valgrind run:

==11343== 45 bytes in 1 blocks are definitely lost in loss record 9 of 20
==11343== at 0x4006AEE: malloc (vg_replace_malloc.c:207)
==11343== by 0x3D847F: strdup (in /lib/libc-2.8.so)
==11343== by 0x40FE2A2: Curl_http_input_auth
==11343== by 0x410F0B2: Curl_readwrite
==11343== by 0x410F3A1: Curl_perform
==11343== by 0x410FFAD: curl_easy_perform
==11343== by 0x4036A41: wsmc_handler (wsman-curl-client-transport.c:438)
==11343== by 0x4035F02: wsman_send_request (wsman-client-transport.c:88)
==11343== by 0x40341F7: wsmc_action_enumerate (wsman-client.c:1436)
==11343== by 0x804AC24: main (wsman.c:758)
==11343==
==11343== LEAK SUMMARY:
==11343== definitely lost: 45 bytes in 1 blocks.
==11343== possibly lost: 0 bytes in 0 blocks.
==11343== still reachable: 3,445 bytes in 110 blocks.
==11343== suppressed: 0 bytes in 0 blocks.
==11343== Reachable blocks (those to which a pointer was found) are not shown.
==11343== To see them, rerun with: --leak-check=full --show-reachable=yes

I've added some debugs and it looks like that the following happens:

- data->req.newurl is allocated the first time in Curl_http_input_auth:
#ifdef HAVE_GSSAPI
  if(checkprefix("GSS-Negotiate", start) ||
      checkprefix("Negotiate", start)) {
    *availp |= CURLAUTH_GSSNEGOTIATE;
    authp->avail |= CURLAUTH_GSSNEGOTIATE;
    if(authp->picked == CURLAUTH_GSSNEGOTIATE) {
      /* if exactly this is wanted, go */
      int neg = Curl_input_negotiate(conn, (bool)(httpcode == 407), start);
      if(neg == 0) {
        DEBUGASSERT(!data->req.newurl);
        data->req.newurl = strdup(data->change.url);

- but req->data.newurl is overwritten later in Curl_http_auth_act:
  if(pickhost || pickproxy) {
    data->req.newurl = strdup(data->change.url); /* clone URL */
without freeing the previously allocated memory

- additionally I've checked this using libcurl's memdebug feature:
 ../curl/tests/memanalyze.pl dump
Leak detected: memory still allocated: 45 bytes
At 9d33e04, there's 45 bytes.
 allocated by ../../lib/http.c:724

- I'm not 100% sure whether the following patch really addresses the root cause of the leak, but it works fine for me:
===================================================================
--- lib/http.c (.../vendor/curl/7.19.2) (revision 186215)
+++ lib/http.c (.../trunk/fwcomponents/curl) (revision 186215)
@@ -458,6 +458,9 @@
   }

   if(pickhost || pickproxy) {
+ if (data->req.newurl) {
+ Curl_safefree(data->req.newurl);
+ }
     data->req.newurl = strdup(data->change.url); /* clone URL */
     if(!data->req.newurl)
       return CURLE_OUT_OF_MEMORY;

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2008-11-19 15:22

Message:
thanks for the test and for the fix, I went and committed your version of
the fix just now

----------------------------------------------------------------------

Comment By: Christian Krause (chkr)
Date: 2008-11-17 14:47

Message:
I'v tried out your suggestion: If I don't set data->req.newurl at this
place then the kerberos authentication stops working completely (because
libcurl won't retry at least one time after it received the 401 reply from
the server after the first (unauthenticated) attempt).

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2008-11-16 13:57

Message:
Thanks for the report!

Could you please check if the reversed modification works, like below?

diff -u -r1.404 http.c
--- http.c 11 Nov 2008 22:19:27 -0000 1.404
+++ http.c 16 Nov 2008 12:56:35 -0000
@@ -715,7 +715,6 @@
       int neg = Curl_input_negotiate(conn, (bool)(httpcode == 407),
start);
       if(neg == 0) {
         DEBUGASSERT(!data->req.newurl);
- data->req.newurl = strdup(data->change.url);
         data->state.authproblem = (data->req.newurl == NULL);
       }
       else {

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2284386&group_id=976
Received on 2008-11-19