#786 memory leak in lib/http.c

closed-fixed
libcurl (356)
5
2013-06-21
2008-11-14
Christian Krause
No

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;

Discussion

  • 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 {

     
  • 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).

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

     
    • status: open --> closed-fixed