From 2e0d8414943c021540e84efb9bd13be9782cb674 Mon Sep 17 00:00:00 2001 From: Andrei Molotkov Date: Wed, 26 Feb 2025 12:11:28 +0000 Subject: [PATCH 1/3] Fix memory leak in ldap auth provider --- ydb/core/security/ldap_auth_provider/ldap_auth_provider.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ydb/core/security/ldap_auth_provider/ldap_auth_provider.cpp b/ydb/core/security/ldap_auth_provider/ldap_auth_provider.cpp index 0b950592df03..3ff313f48400 100644 --- a/ydb/core/security/ldap_auth_provider/ldap_auth_provider.cpp +++ b/ydb/core/security/ldap_auth_provider/ldap_auth_provider.cpp @@ -321,6 +321,7 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped response.Status = NKikimrLdap::ErrorToStatus(result); response.Error = {.Message = ERROR_MESSAGE, .LogMessage = logErrorMessage, .Retryable = NKikimrLdap::IsRetryableError(result)}; LDAP_LOG_D(logErrorMessage); + NKikimrLdap::MsgFree(searchMessage); return response; } const int countEntries = NKikimrLdap::CountEntries(request.Ld, searchMessage); @@ -357,6 +358,7 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped LDAPMessage* searchMessage = nullptr; int result = NKikimrLdap::Search(ld, Settings.GetBaseDn(), NKikimrLdap::EScope::SUBTREE, filter, NKikimrLdap::noAttributes, 0, &searchMessage); if (!NKikimrLdap::IsSuccess(result)) { + NKikimrLdap::MsgFree(searchMessage); return {}; } const int countEntries = NKikimrLdap::CountEntries(ld, searchMessage); @@ -403,6 +405,7 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped LDAPMessage* searchMessage = nullptr; int result = NKikimrLdap::Search(ld, Settings.GetBaseDn(), NKikimrLdap::EScope::SUBTREE, filter, RequestedAttributes, 0, &searchMessage); if (!NKikimrLdap::IsSuccess(result)) { + NKikimrLdap::MsgFree(searchMessage); return; } if (NKikimrLdap::CountEntries(ld, searchMessage) == 0) { From ac7d33d71b9736961d4311c7647c708194a3c555 Mon Sep 17 00:00:00 2001 From: Andrei Molotkov Date: Wed, 26 Feb 2025 13:34:15 +0000 Subject: [PATCH 2/3] Set pointer as nullptr after free memory --- .../security/ldap_auth_provider/ldap_auth_provider.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ydb/core/security/ldap_auth_provider/ldap_auth_provider.cpp b/ydb/core/security/ldap_auth_provider/ldap_auth_provider.cpp index 3ff313f48400..5ab91ce241da 100644 --- a/ydb/core/security/ldap_auth_provider/ldap_auth_provider.cpp +++ b/ydb/core/security/ldap_auth_provider/ldap_auth_provider.cpp @@ -322,6 +322,7 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped response.Error = {.Message = ERROR_MESSAGE, .LogMessage = logErrorMessage, .Retryable = NKikimrLdap::IsRetryableError(result)}; LDAP_LOG_D(logErrorMessage); NKikimrLdap::MsgFree(searchMessage); + searchMessage = nullptr; return response; } const int countEntries = NKikimrLdap::CountEntries(request.Ld, searchMessage); @@ -337,6 +338,7 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped response.Error = {.Message = ERROR_MESSAGE, .LogMessage = logErrorMessage, .Retryable = false}; response.Status = TEvLdapAuthProvider::EStatus::UNAUTHORIZED; NKikimrLdap::MsgFree(searchMessage); + searchMessage = nullptr; LDAP_LOG_D(logErrorMessage); return response; } @@ -359,11 +361,13 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped int result = NKikimrLdap::Search(ld, Settings.GetBaseDn(), NKikimrLdap::EScope::SUBTREE, filter, NKikimrLdap::noAttributes, 0, &searchMessage); if (!NKikimrLdap::IsSuccess(result)) { NKikimrLdap::MsgFree(searchMessage); + searchMessage = nullptr; return {}; } const int countEntries = NKikimrLdap::CountEntries(ld, searchMessage); if (countEntries == 0) { NKikimrLdap::MsgFree(searchMessage); + searchMessage = nullptr; return {}; } std::vector groups; @@ -375,6 +379,7 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped dn = nullptr; } NKikimrLdap::MsgFree(searchMessage); + searchMessage = nullptr; return groups; } @@ -406,10 +411,12 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped int result = NKikimrLdap::Search(ld, Settings.GetBaseDn(), NKikimrLdap::EScope::SUBTREE, filter, RequestedAttributes, 0, &searchMessage); if (!NKikimrLdap::IsSuccess(result)) { NKikimrLdap::MsgFree(searchMessage); + searchMessage = nullptr; return; } if (NKikimrLdap::CountEntries(ld, searchMessage) == 0) { NKikimrLdap::MsgFree(searchMessage); + searchMessage = nullptr; return; } for (LDAPMessage* groupEntry = NKikimrLdap::FirstEntry(ld, searchMessage); groupEntry != nullptr; groupEntry = NKikimrLdap::NextEntry(ld, groupEntry)) { @@ -432,6 +439,7 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped } } NKikimrLdap::MsgFree(searchMessage); + searchMessage = nullptr; } } From 5ffcbc317b052012b11ae2224c1d0092c9c2ea1d Mon Sep 17 00:00:00 2001 From: Andrei Molotkov Date: Wed, 26 Feb 2025 15:13:03 +0000 Subject: [PATCH 3/3] Revert "Set pointer as nullptr after free memory" This reverts commit ac7d33d71b9736961d4311c7647c708194a3c555. --- .../security/ldap_auth_provider/ldap_auth_provider.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/ydb/core/security/ldap_auth_provider/ldap_auth_provider.cpp b/ydb/core/security/ldap_auth_provider/ldap_auth_provider.cpp index 5ab91ce241da..3ff313f48400 100644 --- a/ydb/core/security/ldap_auth_provider/ldap_auth_provider.cpp +++ b/ydb/core/security/ldap_auth_provider/ldap_auth_provider.cpp @@ -322,7 +322,6 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped response.Error = {.Message = ERROR_MESSAGE, .LogMessage = logErrorMessage, .Retryable = NKikimrLdap::IsRetryableError(result)}; LDAP_LOG_D(logErrorMessage); NKikimrLdap::MsgFree(searchMessage); - searchMessage = nullptr; return response; } const int countEntries = NKikimrLdap::CountEntries(request.Ld, searchMessage); @@ -338,7 +337,6 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped response.Error = {.Message = ERROR_MESSAGE, .LogMessage = logErrorMessage, .Retryable = false}; response.Status = TEvLdapAuthProvider::EStatus::UNAUTHORIZED; NKikimrLdap::MsgFree(searchMessage); - searchMessage = nullptr; LDAP_LOG_D(logErrorMessage); return response; } @@ -361,13 +359,11 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped int result = NKikimrLdap::Search(ld, Settings.GetBaseDn(), NKikimrLdap::EScope::SUBTREE, filter, NKikimrLdap::noAttributes, 0, &searchMessage); if (!NKikimrLdap::IsSuccess(result)) { NKikimrLdap::MsgFree(searchMessage); - searchMessage = nullptr; return {}; } const int countEntries = NKikimrLdap::CountEntries(ld, searchMessage); if (countEntries == 0) { NKikimrLdap::MsgFree(searchMessage); - searchMessage = nullptr; return {}; } std::vector groups; @@ -379,7 +375,6 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped dn = nullptr; } NKikimrLdap::MsgFree(searchMessage); - searchMessage = nullptr; return groups; } @@ -411,12 +406,10 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped int result = NKikimrLdap::Search(ld, Settings.GetBaseDn(), NKikimrLdap::EScope::SUBTREE, filter, RequestedAttributes, 0, &searchMessage); if (!NKikimrLdap::IsSuccess(result)) { NKikimrLdap::MsgFree(searchMessage); - searchMessage = nullptr; return; } if (NKikimrLdap::CountEntries(ld, searchMessage) == 0) { NKikimrLdap::MsgFree(searchMessage); - searchMessage = nullptr; return; } for (LDAPMessage* groupEntry = NKikimrLdap::FirstEntry(ld, searchMessage); groupEntry != nullptr; groupEntry = NKikimrLdap::NextEntry(ld, groupEntry)) { @@ -439,7 +432,6 @@ class TLdapAuthProvider : public NActors::TActorBootstrapped } } NKikimrLdap::MsgFree(searchMessage); - searchMessage = nullptr; } }