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

Issue ID: 10591 Summary: slapo-syncprov frees modtarget and sessionlog nodes without checking that they were removed from their AVL tree, causing a use-after-free and SIGSEGV in the comparison callback Product: OpenLDAP Version: unspecified Hardware: All OS: All Status: UNCONFIRMED Keywords: needs_review Severity: normal Priority: --- Component: overlays Assignee: bugs@openldap.org Reporter: bohdan.kmit@kiteworks.com Target Milestone: ---

## Symptom

slapd terminates with SIGSEGV. Across 89 coredumps collected from several instances, 64 abort inside `malloc()` and 18 inside `free()` at unrelated allocation sites — glibc detecting an already-corrupt heap at whatever the process allocated next, reported variously as:

``` double free or corruption (out) malloc(): unaligned tcache chunk detected malloc(): unaligned fastbin chunk detected ```

Two cores caught the fault itself rather than a later detection. Both have no allocator frame and are structurally identical:

``` #0 sp_avl_cmp (c1=0x7f5778104590, c2=0x7f5f4954e13a) at syncprov.c:433 #1 ldap_avl_delete (root=..., data=c1, fcmp=sp_avl_cmp) at avl.c:199 #2 syncprov_op_cleanup (op, rs) at syncprov.c:1589 #3 slap_cleanup_play at result.c:607 #4 send_ldap_response at result.c:797 #5 slap_send_ldap_result at result.c:926 #6 mdb_add at add.c:389 #7 overlay_op_walk (which=op_add) at backover.c:706 #9 accesslog_response at accesslog.c:1966 #10 slap_response_play at result.c:573 #13 mdb_modify at modify.c:803 #16 syncrepl_message_to_op at syncrepl.c:3271 #17 do_syncrep2 at syncrepl.c:1555 #18 do_syncrepl at syncrepl.c:2197 ```

`si_addr` is `c2 + 0x10` in both. `modtarget` is

```c typedef struct modtarget { struct modinst *mt_mods; /* +0 */ struct modinst *mt_tail; /* +8 */ struct berval mt_dn; /* +16 -> bv_len */ ldap_pvt_thread_mutex_t mt_mutex; } modtarget; ```

so offset 16 is `mt_dn.bv_len`, exactly the field `sp_avl_cmp` reads at syncprov.c:433. Both `c2` values are not 8-byte aligned, so `si_mods` is holding freed and reused memory rather than a live `modtarget`.

The path is a replicated MODIFY applied by syncrepl, whose response triggers the accesslog overlay to perform a nested internal ADD into the log database; that nested operation's cleanup walks `si_mods` and dereferences the stale node.

## Analysis

`syncprov_op_cleanup()`, syncprov.c:

```c ldap_avl_delete( &si->si_mods, mt, sp_avl_cmp ); /* return value ignored */ ldap_pvt_thread_mutex_unlock( &si->si_mods_mutex ); ldap_pvt_thread_mutex_destroy( &mt->mt_mutex ); ch_free( mt->mt_dn.bv_val ); ch_free( mt ); ```

`ldap_avl_delete()` returns the node it removed, or NULL when the comparison does not locate it. Since `sp_avl_cmp` orders by `mt_dn`, a search can fail to find a target that is physically still in the tree, and can also match a different target that shares a DN. In either case `mt` is freed while still linked, and the next traversal dereferences it.

Access to the tree is correctly serialised at all three sites (`ldap_avl_find` at :2787, `ldap_avl_insert` at :2861, `ldap_avl_delete` at :1589 all under `si_mods_mutex`), so this is a lifetime defect rather than a data race.

Three related weaknesses in the same overlay:

1. `ldap_avl_insert()` at syncprov.c:2861 also ignores its return. A failed insert leaves `mt` unreferenced by the tree while `opc->smt` still points at it, so the later cleanup finds nothing to remove.

2. The `mt_mods` walk at syncprov.c:1574 and the `mt_mods` and `o_callback` walks in the abandon path at syncprov.c:2848 and :2856 have no termination condition:

```c for (m2 = &mt->mt_mods; ; m2 = &(*m2)->mi_next) { ```

If the entry is not on the list the loop runs off the end. This is the same pattern ITS#10408 corrected for `si_ops`, which was released in 2.6.15; the equivalent code two functions away was not changed.

3. The sessionlog trim at syncprov.c:1799 has the identical delete-then-free shape on a different tree:

```c ldap_tavl_delete( &sl->sl_entries, se, syncprov_sessionlog_cmp ); ch_free( se ); ```

One of the collected cores aborts in `syncprov_add_slog()` -> `ldap_tavl_insert()` at syncprov.c:1754, consistent with that tree also holding a freed node. The sessionlog is written on every logged operation, so on a busy provider it is exercised harder than `si_mods`.

## Same defect in two other overlays

Found while auditing for the pattern; neither was loaded when the crash was observed, so these are reported from code inspection only.

`slapo-pcache`, `remove_from_template()`: `ldap_avl_delete()` on `template->qbase` is unchecked and followed immediately by `ch_free( qc->qbase )`. The `ldap_tavl_delete()` on the scope tree above it is also unchecked, and both callers free `qc` only later, so a failed removal leaves either tree pointing at freed memory.

`slapo-seqmod`, `seqmod_op_cleanup()`: the lookup result is validated with `assert( av != NULL )` and then dereferenced as `av->avl_data`. Under NDEBUG the assert is compiled out and the dereference faults instead. Its `ldap_avl_delete()` is also unchecked, though nothing is freed on that path.

An audit of `servers/slapd` found no other unguarded `for (p = &head; ; p = &(*p)->next)` list walks, and `back-ldap/chain.c:1506` already checks its `ldap_tavl_delete()` return and logs on failure.

## Affected versions

Verified byte-identical in `OPENLDAP_REL_ENG_2_6_15`, `OPENLDAP_REL_ENG_2_7_0`, `OPENLDAP_REL_ENG_2_7_1` and current `master`. Diffing `syncprov.c` between 2.6.15 and master produces no hunk touching `si_mods`, `modtarget`, `sp_avl_cmp`, `mt_mods`, `mt_tail` or `opc->smt`, and `avl.c` is unchanged. So there is no release to upgrade to.

Crash observed on 2.6.15, x86-64, glibc, back-mdb, overlays `syncprov` and `accesslog` loaded, three-way multi-provider mesh with delta-syncrepl consumers reading the accesslog. Sustained write load of roughly 25 to 30 operations per second.

## Proposed fix

Three patches, one per overlay:

1. **slapo-syncprov** — capture the return of `ldap_avl_delete()` and `ldap_tavl_delete()` and free only on an identity match, logging otherwise; bound the three list walks; report a failed `ldap_avl_insert()`. This converts a use-after-free into a bounded leak: a leaked `modtarget` is a few dozen bytes plus a DN, a dangling one takes the process down.

2. **slapo-pcache** — same guard on both removals in `remove_from_template()`.

3. **slapo-seqmod** — replace the assert with a real check that releases the mutex, completes the callback teardown and returns; report a failed `ldap_avl_delete()`.