From 5468cd49dadbf211e50cff2d6738da751fc8e819 Mon Sep 17 00:00:00 2001 From: Marcin Maliszkiewicz Date: Tue, 7 Apr 2026 17:27:43 +0200 Subject: [PATCH] Merge 'ldap: fix double-free of LDAPMessage in poll_results()' from Andrzej Jackowski In the unregistered-ID branch, ldap_msgfree() was called on a result already owned by an RAII ldap_msg_ptr, causing a double-free on scope exit. Remove the redundant manual free. Fixes: SCYLLADB-1344 Backport: 2026.1, 2025.4, 2025.1 - it's a memory corruption, with a one-line fix, so better backport it everywhere. Closes scylladb/scylladb#29302 * github.com:scylladb/scylladb: test: ldap: add regression test for double-free on unregistered message ID ldap: fix double-free of LDAPMessage in poll_results() (cherry picked from commit 895fdb6d2934a86432314ca6ebf6ebc66a24dcd7) Closes scylladb/scylladb#29393 Closes scylladb/scylladb#29454 --- ent/ldap/ldap_connection.cc | 1 - test/ldap/ldap_connection_test.cc | 30 +++++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/ent/ldap/ldap_connection.cc b/ent/ldap/ldap_connection.cc index fb5134d49b..a333d6aeba 100644 --- a/ent/ldap/ldap_connection.cc +++ b/ent/ldap/ldap_connection.cc @@ -437,7 +437,6 @@ void ldap_connection::poll_results() { const auto found = _msgid_to_promise.find(id); if (found == _msgid_to_promise.end()) { mylog.error("poll_results: got valid result for unregistered id {}, dropping it", id); - ldap_msgfree(result); } else { found->second.set_value(std::move(result_ptr)); _msgid_to_promise.erase(found); diff --git a/test/ldap/ldap_connection_test.cc b/test/ldap/ldap_connection_test.cc index b8b2d502b1..4b84100cc9 100644 --- a/test/ldap/ldap_connection_test.cc +++ b/test/ldap/ldap_connection_test.cc @@ -235,7 +235,7 @@ SEASTAR_THREAD_TEST_CASE(multiple_outstanding_operations_on_failing_connection) mylog.trace("multiple_outstanding_operations_on_failing_connection"); with_ldap_connection(local_fail_inject_address, [] (ldap_connection& c) { mylog.trace("multiple_outstanding_operations_on_failing_connection: invoking bind"); - bind(c).handle_exception(&ignore).get();; + bind(c).handle_exception(&ignore).get(); std::vector> results_base; for (size_t i = 0; i < 10; ++i) { @@ -291,3 +291,31 @@ SEASTAR_THREAD_TEST_CASE(severed_connection_yields_exceptional_future) { } }); } + +// Requires ASAN or valgrind to reliably detect the double-free. +SEASTAR_THREAD_TEST_CASE(unregistered_msgid_double_free) { + set_defbase(); + with_ldap_connection(local_ldap_address, [] (ldap_connection& c) { + const auto bind_res = bind(c).get(); + BOOST_REQUIRE_EQUAL(LDAP_RES_BIND, ldap_msgtype(bind_res.get())); + + // Bypass the public API to send a search without registering its + // message ID, so poll_results() hits the unregistered-ID branch. + int msgid = -1; + const int rc = ldap_search_ext(c.get_ldap(), const_cast(base_dn), LDAP_SCOPE_SUBTREE, + /*filter=*/nullptr, + /*attrs=*/nullptr, + /*attrsonly=*/0, + /*serverctrls=*/nullptr, + /*clientctrls=*/nullptr, + /*timeout=*/nullptr, + /*sizelimit=*/0, &msgid); + BOOST_REQUIRE_EQUAL(LDAP_SUCCESS, rc); + BOOST_REQUIRE_NE(-1, msgid); + + // A public-API search forces poll_results() to process the + // unregistered response before returning. + const auto dummy = search(c, base_dn).get(); + BOOST_REQUIRE(dummy.get()); + }); +}