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

Issue ID: 10597 Summary: slapo-accesslog frees a shared `entryUUID` value from operations that do not hold the mutex guarding it, causing a double free and heap corruption. Introduced in 2.6.14 by the fix for ITS#10482. Product: OpenLDAP Version: 2.6.14 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: ---

Created attachment 1212 --> https://bugs.openldap.org/attachment.cgi?id=1212&action=edit Patch

# Symptom

`slapd` terminates with SIGSEGV under concurrent write and search load. glibc reports the corruption at whatever the process allocates next, so the message varies:

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

Of ten cores collected from one instance, nine abort inside `malloc()` or `calloc()` at unrelated allocation sites and carry no information about the origin. One caught the faulting free itself:

``` #4 free () #5 accesslog_response (op=<optimized out>, rs=...) at accesslog.c:2034 #6 ... #8 slap_send_ldap_result () ```

(line 2034 in 2.6.15; the equivalent is 2176 in current master.)

# Analysis

`accesslog_response()` ends with:

```c skip: if ( !BER_BVISNULL( &li->li_uuid ) ) { ber_memfree( li->li_uuid.bv_val ); BER_BVZERO( &li->li_uuid ); } if ( lo->mask & LOG_OP_WRITES ) { /* We haven't transitioned to li_log_mutex yet */ ldap_pvt_thread_mutex_unlock( &li->li_op_rmutex ); } return SLAP_CB_CONTINUE; ```

`li_uuid` is a field of the shared `log_info` instance, not of the operation. Access to it is asymmetric:

* Write operations (`bi_op_add`, `bi_op_delete`, `bi_op_modify`, `bi_op_modrdn` → `accesslog_op_mod()`) acquire `li_op_rmutex` and populate `li_uuid` with `ber_dupbv()` while holding it. They still hold it on entry to `accesslog_response()`, free under it at `skip:`, and release it immediately after. This is correct.

* Non-write operations (`bi_op_bind`, `bi_op_compare`, `bi_op_search`, `bi_extended` → `accesslog_op_misc()`) take no lock at all. `accesslog_response()` acquires `li_op_rmutex` for them only part-way through the function, and **all four `goto skip` sites precede that acquisition** (master: 1656, 1661, 1671, 1683 versus the lock at 1693). Such an operation therefore reaches `skip:` holding no lock, and frees a pointer it never set.

The `goto skip` conditions reachable this way are:

| site | condition | |------|-----------------------------------------------------| | 1656 | log database absent or not open | | 1661 | `op->o_dont_replicate` | | 1671 | `li_success` configured and the operation failed | | 1683 | operation not in `li_ops` and no matching `logbase` |

Two consequences follow:

1. Two non-write operations completing concurrently both observe a non-NULL `li_uuid` and both call `ber_memfree()` on it — a double free. 2. A non-write operation can free it in the window between a write operation storing the value and that write operation taking ownership of it (master 1716–1718, under both mutexes). The write path then works with, and frees, a dangling pointer.

Either way the allocator's free lists are corrupted, and the failure surfaces later at an unrelated allocation, which is why almost every core points somewhere innocent.

The exposure depends strongly on configuration. With `olcAccessLogOps: writes`, searches are not in `li_ops`, so **every search** fails the test at 1673 and takes `goto skip` at 1683. A read-heavy workload therefore executes the unlocked free at close to search rate. Note that 1683 is unreachable for write operations, since `accesslog_op_mod()` applies the same test before registering the callback.

Access to the tree of related state is otherwise correct: every use of `li_mincsn`, `li_sids` and `li_numcsns` in `log_old_lookup()`, `accesslog_purge()`, `accesslog_response()` and `accesslog_db_root()` is serialised by `li_log_mutex`, and no locked region in the file contains a `return` or `goto` that bypasses its unlock. `li_uuid` is the only unserialised mutation.

# Regression

The unconditional free was added by:

``` 5c4e7f2f1a ITS#10482 slapo-accesslog: do not leak entryUUID (2026-03-31) ab4e53e54b same, backported to the 2.6 branch ```

That commit consists solely of those four lines. It plugs a genuine leak but places the release on a path reachable without the mutex.

# Affected versions

| release | affected | |--------------|----------| | 2.6.13 | no | | 2.6.14 | yes | | 2.6.15 | yes | | 2.7.0, 2.7.1 | yes | | master | yes |

Observed on 2.6.15, x86-64, glibc, back-mdb, with `syncprov` and `accesslog` on the same database and delta-syncrepl consumers reading the accesslog.

# Reproduction

Configuration: `accesslog` over back-mdb with `olcAccessLogOps: writes` and `olcAccessLogSuccess: TRUE`.

Load: six concurrent MODIFY streams cycling over four DNs (to maximise same-target concurrency), two more spread over twenty DNs, one stream of ADDs of existing entries (returning `entryAlreadyExists`), and twelve concurrent search streams over the same subtree.

Result: SIGSEGV within **13–24 seconds**, repeatably — effectively one crash per load application.

Two observations that isolate the mechanism:

* The **same write load with no search streams ran 300 seconds without a fault.** Searches are required, because only non-write operations take the unlocked path. * Instances receiving the identical replicated write stream but carrying **no local search traffic** ran 68 minutes without a fault, while an instance with local searches plus the same replicated writes crashed.

`MALLOC_CHECK_` does not help: by the time of the second free the chunk has typically been handed to another allocation, so it is a valid live chunk and nothing is flagged at the free.

# Proposed fix

Free the value only on the path that owns it and already holds the mutex:

```diff skip: - if ( !BER_BVISNULL( &li->li_uuid ) ) { - ber_memfree( li->li_uuid.bv_val ); - BER_BVZERO( &li->li_uuid ); - } if ( lo->mask & LOG_OP_WRITES ) { + /* Only this path holds li_op_rmutex, which guards li_uuid */ + if ( !BER_BVISNULL( &li->li_uuid ) ) { + ber_memfree( li->li_uuid.bv_val ); + BER_BVZERO( &li->li_uuid ); + } /* We haven't transitioned to li_log_mutex yet */ ldap_pvt_thread_mutex_unlock( &li->li_op_rmutex ); } ```

Non-write operations never populate `li_uuid`, so they have nothing to release; the leak ITS#10482 addressed remains fixed for the write path, which is the only producer. Because every `goto skip` precedes the mutex acquisition, no path can now reach the label holding the lock without releasing it.

# Related observation, not addressed by this patch

`li_old` has the same missing release. `accesslog_op_mod()` assigns `li->li_old = entry_dup( e )` without freeing any previous value, `accesslog_response()` moves it out only on the non-skip path, and neither `skip:` nor `accesslog_db_destroy()` releases it. A write operation whose response takes `goto skip` therefore strands an `Entry`, which the next write overwrites and leaks. This requires `olcAccessLogOld` to be configured and is a leak only — no unserialised free, so no corruption. Reported for completeness rather than fixed here.