Ooops, you were right Peter.
I've checked the function with a memory monitor, and as the variable names
suggests it frees the memory when the 'freeit' value is 1.
So, the text of the ldap_result2error() is wrong on manual, eh?
Anyway, then the leak is still somewhere else.
Regards,
---
Juan
----- Original Message -----
From: "Juan Marchionatto" <[EMAIL PROTECTED]>
To: <[EMAIL PROTECTED]>
Sent: Tuesday, September 04, 2001 10:27 AM
Subject: Re: (Freeradius 0.1 vs. Freeradius 0.2) + LDAP
Peter,
as you transcripted from the result2error ldap call
int ldap_result2error(LDAP *ld, LDAPMessage *res, int freeit);
* If the freeit parameter is non zero it indicates
* that the res parameter should be freed by a call
* to ldap_msgfree(3) after the error code has been extracted.
This means the 1 will NOT free the memory. Isn't it?
So this must be a leak source after all, as far as I can see.
And regarding the ldap_connect routine being called only at startup time,
this is not the only opportunity it is called, if I followed the code well
(which may be not, because I only did a visual check, as I don't have the
server running yet) .
Regards,
---
Juan
----- Original Message -----
From: "Peter Foreman" <[EMAIL PROTECTED]>
To: <[EMAIL PROTECTED]>
Sent: Monday, September 03, 2001 4:03 AM
Subject: RE: (Freeradius 0.1 vs. Freeradius 0.2) + LDAP
Not exactly. From the ldap_result man page:
Upon success, the type of the result received is returned
and the result parameter will contain the result of the
operation. This result should be passed to the LDAP pars�
ing routines, ldap_first_entry(3) and friends, for inter�
pretation.
ERRORS
ldap_result() returns -1 if something bad happens, and
zero if the timeout specified was exceeded.
That means that the first ldap_msgfree which you introduced is wrong, since
rc < 1 and that means that there was no success (and no res allocated
either).
For the second one, again from the manpage:
The ldap_result2error() routine takes res, a result as
produced by ldap_result(3) or ldap_search_s(3), and
returns the corresponding error code. Possible error
codes are listed below. If the freeit parameter is non
zero it indicates that the res parameter should be freed
by a call to ldap_msgfree(3) after the error code has been
extracted. The ld_errno field in ld is set and returned.
And if you look at the code:
ldap_errno = ldap_result2error(ld, res, 1);
The '1' tells ldap_result2error to free the res parameter. Also the second
ldap_msgfree you introduced should not be there!
/Peter
-----Original Message-----
From: John Morrissey [mailto:[EMAIL PROTECTED]]
Sent: Saturday, September 01, 2001 5:46 PM
To: [EMAIL PROTECTED]
Subject: Re: (Freeradius 0.1 vs. Freeradius 0.2) + LDAP
On Fri, Aug 31, 2001 at 10:34:15AM -0400, Juan Marchionatto wrote:
% Alan, Peter,
% am I missing something (again) or the ldap_connect routine should
% ldap_msgfree(res) before returning?
%
% This may be specially important when returning OK, because there is not
even
% an ldap_unbind (which might eventually free the memory) in that case.
You're right; the result passed to ldap_result() needs to be freed by the
caller. The diff below frees them properly.
I also read the source to rlm_ldap (albeit not as thoroughly as I'd like)
and couldn't find any more situations like this. Somehow I doubt that this
is the only source of the LDAP leaks I've seen reported. ldap_connect() only
gets called when the module is first initialized and when the LDAP server
goes away, so unless your LDAP server is bouncing like a rubber ball, I
don't see how this could account for the leaks I recall hearing about.
john
Index: rlm_ldap.c
===================================================================
RCS file: /source/radiusd/src/modules/rlm_ldap/rlm_ldap.c,v
retrieving revision 1.50
diff -u -r1.50 rlm_ldap.c
--- rlm_ldap.c 2001/08/28 20:01:48 1.50
+++ rlm_ldap.c 2001/09/01 15:28:41
@@ -640,10 +641,12 @@
ldap_get_option(ld, LDAP_OPT_ERROR_NUMBER, &ldap_errno);
radlog(L_ERR, "rlm_ldap: %s bind failed: %s", dn, (rc == 0)
? "timeout" : ldap_err2string(ldap_errno));
*result = RLM_MODULE_FAIL;
+ ldap_msgfree(res);
ldap_unbind_s(ld);
return (NULL);
}
ldap_errno = ldap_result2error(ld, res, 1);
+ ldap_msgfree(res);
switch (ldap_errno) {
case LDAP_SUCCESS:
*result = RLM_MODULE_OK;
--
John Morrissey _o /\ ---- __o
[EMAIL PROTECTED] _-< \_ / \ ---- < \,
www.horde.net/ __(_)/_(_)________/ \_______(_) /_(_)__
-
List info/subscribe/unsubscribe? See
http://www.freeradius.org/list/users.html
-
List info/subscribe/unsubscribe? See
http://www.freeradius.org/list/users.html
-
List info/subscribe/unsubscribe? See
http://www.freeradius.org/list/users.html
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
