Memory leak in ntlm_auth ?
Stefan Metzmacher
metze at samba.org
Tue Apr 4 09:57:12 UTC 2017
Hi Rebecca,
thanks for the report,
can you please also file a bug at https://bugzilla.samba.org,
this is required to get the fix into the release branches.
While I think your patch fixes the problem, I'd propose the attached
patch for upstream.
Can you test if the attached patch also works for you?
Thanks!
metze
Am 04.04.2017 um 11:20 schrieb Rebecca Gellman via samba-technical:
>
>
> Hi,
>
> I wish to tell you a tale of investigation and suspicion of a memory
> leak in the ntlm_auth program (as compiled from
> source3/utils/ntlm_auth.c).
>
> So in our system, we use ntlm_auth to provide both Kerberos and NTLM
> authentication for transparent (and not-so transparent) web proxy
> connections. In this particular instance, Kerberos is the auth of
> choice, but I believe this is relevant to NTLM too.
>
> We spawn up a pool of ntlm_auth processes on demand. If a process is not
> available when a connection comes in, we create a new one up to a
> maximum.
>
> The base software version is 4.2.10, as shipped with Debian 8 (Jessie).
>
> Now that the particulars of the environment are out of the way.....
>
> So we have a customer. And they phoned in a support call that their
> system was falling over. We dive in remotely, and see each ntlm_auth
> process is running at an impressive 1.7GB of memory in the "VIRT" column
> of top. EEP!
>
> Suspecting a memory leak, we compiled up version 4.2.10 with a call to
> talloc_enable_leak_reporting_full() in source3/utils/ntlm_auth.c (why
> isn't this a command line option at the least?), along with a mod to our
> own processes to send CTRL+D to ntlm_auth prior to terminating the
> process, and gathering the report on STDERR.
>
> Left it running for a day and examined the report. Each process had on
> average over 19,000 DATA_BLOBs of around 1.5K in size, reportedly
> created at lib/util/base64.c line 35, the function
> base64_decode_data_blob_talloc().
>
> After digging through the code, this function is called on (pseudo-code)
> &line_of_STDIN[3] to decode the base64 blob input to a binary blob.
> There's then a load of logic surrounding the 2-letter command code and
> processing gensec, etc, etc, etc.
>
> Now... the crux of the matter. If I compare with an earlier (3.x.x)
> branch, we see that data_blob_free() is called on this returned blob
> whenever the command handler returns. In 4.x.x this code has been
> refactored, and the function manage_gensec_request() doesn't always call
> data_blob_free(). In fact, it rarely calls it at all.
>
> There's also a couple of returns that omit talloc_free(mem_ctx) that
> seems to precede every other return in this function.
>
> So, on this hunch, I added in the missing data_blob_free() calls and the
> two talloc_free(mem_ctx) calls, and rebuilt. Now the same customer
> system is sitting happily at a low memory usage, not showing so much of
> a hint of a memory leak.
>
> I've attached the patch we used to "correct" ntlm_auth.c. I'd be
> interested to here some technical commentary from knowledge Samba bods
> :)
>
> Regards
>
> Rebecca Gellman (On behalf of SmoothWall)
>
>
>
-------------- next part --------------
From 84ef966e4a33fd770ea35dafb10da5c8bd0a72a9 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 4 Apr 2017 11:52:56 +0200
Subject: [PATCH] s3:ntlm_auth: fix memory leak in manage_gensec_request()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source3/utils/ntlm_auth.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/source3/utils/ntlm_auth.c b/source3/utils/ntlm_auth.c
index 6a7a269..5a10e27 100644
--- a/source3/utils/ntlm_auth.c
+++ b/source3/utils/ntlm_auth.c
@@ -1306,6 +1306,8 @@ static void manage_gensec_request(enum stdio_helper_mode stdio_helper_mode,
TALLOC_CTX *mem_ctx;
+ mem_ctx = talloc_named(NULL, 0, "manage_gensec_request internal mem_ctx");
+
if (*private1) {
state = (struct gensec_ntlm_state *)*private1;
} else {
@@ -1323,6 +1325,7 @@ static void manage_gensec_request(enum stdio_helper_mode stdio_helper_mode,
if (strlen(buf) < 2) {
DEBUG(1, ("query [%s] invalid", buf));
printf("BH Query invalid\n");
+ talloc_free(mem_ctx);
return;
}
@@ -1332,9 +1335,10 @@ static void manage_gensec_request(enum stdio_helper_mode stdio_helper_mode,
talloc_free(want_feature_list);
want_feature_list = talloc_strndup(state, buf+3, strlen(buf)-3);
printf("OK\n");
+ talloc_free(mem_ctx);
return;
}
- in = base64_decode_data_blob(buf + 3);
+ in = base64_decode_data_blob_talloc(mem_ctx, buf + 3);
} else {
in = data_blob(NULL, 0);
}
@@ -1347,7 +1351,7 @@ static void manage_gensec_request(enum stdio_helper_mode stdio_helper_mode,
} else if ( (strncmp(buf, "OK", 2) == 0)) {
/* Just return BH, like ntlm_auth from Samba 3 does. */
printf("BH Command expected\n");
- data_blob_free(&in);
+ talloc_free(mem_ctx);
return;
} else if ( (strncmp(buf, "TT ", 3) != 0) &&
(strncmp(buf, "KK ", 3) != 0) &&
@@ -1359,12 +1363,10 @@ static void manage_gensec_request(enum stdio_helper_mode stdio_helper_mode,
(strncmp(buf, "GF", 2) != 0)) {
DEBUG(1, ("SPNEGO request [%s] invalid prefix\n", buf));
printf("BH SPNEGO request invalid prefix\n");
- data_blob_free(&in);
+ talloc_free(mem_ctx);
return;
}
- mem_ctx = talloc_named(NULL, 0, "manage_gensec_request internal mem_ctx");
-
/* setup gensec */
if (!(state->gensec_state)) {
switch (stdio_helper_mode) {
@@ -1499,7 +1501,6 @@ static void manage_gensec_request(enum stdio_helper_mode stdio_helper_mode,
state->set_password,
CRED_SPECIFIED);
printf("OK\n");
- data_blob_free(&in);
talloc_free(mem_ctx);
return;
}
@@ -1531,10 +1532,12 @@ static void manage_gensec_request(enum stdio_helper_mode stdio_helper_mode,
neg_flags = gensec_ntlmssp_neg_flags(state->gensec_state);
if (neg_flags == 0) {
printf("BH\n");
+ talloc_free(mem_ctx);
return;
}
printf("GF 0x%08x\n", neg_flags);
+ talloc_free(mem_ctx);
return;
}
--
1.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170404/8cf5bd9f/signature.sig>
More information about the samba-technical
mailing list