https://bugs.openldap.org/show_bug.cgi?id=10450

--- Comment #2 from KY <[email protected]> ---
Hi,
I need to issue a correction to my previous report regarding the parseAssert()
memory leak fix. After further investigation, I discovered that the change I
proposed was not actually fixing the leak - I apologize for the confusion.

What Went Wrong
The cleanup code I removed from parseAssert():


if( op->o_assertion != NULL ) {
    filter_free_x( op, op->o_assertion, 1 );
    op->o_assertion = NULL;
}
Was actually dead code that never executed. I incorrectly assumed it was the
fix, but:

get_filter() only sets op->o_assertion on success (filter.c:312)
On SLAPD_DISCONNECT errors, op->o_assertion remains NULL
The cleanup condition was never true, so nothing was ever freed
I should have traced the allocation more carefully before concluding where the
fix belonged. My apologies for the oversight.

The Actual Fix
The real leak occurs in get_filter_list() where partially-allocated filter
siblings are orphaned when a later child fails. The correct fix is to clean up
at the source:

File: servers/slapd/filter.c

Function: get_filter_list()


diff --git a/servers/slapd/filter.c b/servers/slapd/filter.c
index 18970a4..0565ff8 100644
--- a/servers/slapd/filter.c
+++ b/servers/slapd/filter.c
@@ -336,20 +336,29 @@ get_filter_list( Operation *op, BerElement *ber,
        int depth )
 {
        Filter          **new;
+       Filter          *p, *next;
        int             err;
        ber_tag_t       tag;
        ber_len_t       len;
        char            *last;

        Debug( LDAP_DEBUG_FILTER, "begin get_filter_list\n" );
+       *f = NULL;
        new = f;
        for ( tag = ber_first_element( ber, &len, &last );
                tag != LBER_DEFAULT;
                tag = ber_next_element( ber, &len, last ) )
        {
                err = get_filter0( op, ber, new, text, depth );
-               if ( err != LDAP_SUCCESS )
+               if ( err != LDAP_SUCCESS ) {
+                       /* Free already-allocated filters in the list */
+                       for ( p = *f; p != NULL; p = next ) {
+                               next = p->f_next;
+                               filter_free_x( op, p, 1 );
+                       }
+                       *f = NULL;
                        return( err );
+               }
                new = &(*new)->f_next;
        }
        *new = NULL;
This fix has been verified to resolve the ASAN leak report.

Again, I apologize for the earlier misdirection. Please let me know if you have
any questions about this correction.

-- 
You are receiving this mail because:
You are on the CC list for the issue.

Reply via email to