From 95ba82e0a743d0fb0578779335dd711779299733 Mon Sep 17 00:00:00 2001 From: Niels Alexander Buegel Date: Wed, 26 Nov 2025 23:54:22 +0100 Subject: [PATCH 1/3] Objectstore memory leak fixes --- objectstore/cta-objectstore-collect-orphaned-object.cpp | 4 +++- .../cta-objectstore-create-missing-repack-index.cpp | 8 +++++--- .../cta-objectstore-dereference-removed-queues.cpp | 4 +++- objectstore/cta-objectstore-dump-object.cpp | 6 ++++-- objectstore/cta-objectstore-initialize.cpp | 4 +++- objectstore/cta-objectstore-list.cpp | 4 +++- 6 files changed, 21 insertions(+), 9 deletions(-) diff --git a/objectstore/cta-objectstore-collect-orphaned-object.cpp b/objectstore/cta-objectstore-collect-orphaned-object.cpp index 8bdc1fada2..60335aa6a7 100644 --- a/objectstore/cta-objectstore-collect-orphaned-object.cpp +++ b/objectstore/cta-objectstore-collect-orphaned-object.cpp @@ -77,7 +77,9 @@ int main(int argc, char ** argv) { try { dynamic_cast(*be).noDeleteOnExit(); } catch (std::bad_cast &){} - std::cout << "Object store path: " << be->getParams()->toURL() << std::endl; + auto params = be->getParams(); + [[maybe_unused]] std::unique_ptr paramsCleanupPtr(params); // Ensures the params pointer is always cleaned up correctly + std::cout << "Object store path: " << params->toURL() << std::endl; // Try and open the object. cta::objectstore::GenericObject go(objectName, *be); cta::objectstore::ScopedExclusiveLock gol(go); diff --git a/objectstore/cta-objectstore-create-missing-repack-index.cpp b/objectstore/cta-objectstore-create-missing-repack-index.cpp index 1f02300fb7..19fdb3f318 100644 --- a/objectstore/cta-objectstore-create-missing-repack-index.cpp +++ b/objectstore/cta-objectstore-create-missing-repack-index.cpp @@ -16,7 +16,7 @@ */ /** - * This program will make sure every queue listed in the root entry does exist and + * This program will make sure every queue listed in the root entry does exist and * will remove reference for the ones that do not. This utility was created to quickly * unblock tape servers after changing the ArchiveQueue schema during development. */ @@ -55,7 +55,9 @@ int main(int argc, char ** argv) { try { dynamic_cast(*be).noDeleteOnExit(); } catch (std::bad_cast &){} - std::cout << "Object store path: " << be->getParams()->toURL() << std::endl; + auto params = be->getParams(); + [[maybe_unused]] std::unique_ptr paramsCleanupPtr(params); // Ensures the params pointer is always cleaned up correctly + std::cout << "Object store path: " << params->toURL() << std::endl; // Open the root entry RW std::cout << "Creating AgentReference for the creation of the repack index" << std::endl; cta::objectstore::AgentReference agr("cta-objectstore-create-missing-repack-index", logger); @@ -75,7 +77,7 @@ int main(int argc, char ** argv) { std::cout << "Trying to insert repack index" << std::endl; std::string repackIndexAddress = re.addOrGetRepackIndexAndCommit(agr); std::cout << "Repack index created. Address = " << repackIndexAddress << std::endl; - + } catch (const cta::objectstore::RootEntry::DriveRegisterNotEmpty &ex ) { std::cout << "Could not remove the already existing repack index, errorMsg = " << ex.getMessageValue(); return 1; diff --git a/objectstore/cta-objectstore-dereference-removed-queues.cpp b/objectstore/cta-objectstore-dereference-removed-queues.cpp index 9aa2acc36a..b62f2ac6f0 100644 --- a/objectstore/cta-objectstore-dereference-removed-queues.cpp +++ b/objectstore/cta-objectstore-dereference-removed-queues.cpp @@ -102,7 +102,9 @@ int main(int argc, char ** argv) { try { dynamic_cast(*be).noDeleteOnExit(); } catch (std::bad_cast &){} - std::cout << "Object store path: " << be->getParams()->toURL() << std::endl; + auto params = be->getParams(); + [[maybe_unused]] std::unique_ptr paramsCleanupPtr(params); // Ensures the params pointer is always cleaned up correctly + std::cout << "Object store path: " << params->toURL() << std::endl; // Open the root entry RW cta::objectstore::RootEntry re(*be); cta::objectstore::ScopedExclusiveLock rel(re); diff --git a/objectstore/cta-objectstore-dump-object.cpp b/objectstore/cta-objectstore-dump-object.cpp index e9ddaa3b3a..722ad3bb7e 100644 --- a/objectstore/cta-objectstore-dump-object.cpp +++ b/objectstore/cta-objectstore-dump-object.cpp @@ -102,7 +102,9 @@ int main(int argc, char ** argv) { } catch (std::bad_cast &){} cta::objectstore::GenericObject ge(objectName, *be); ge.fetchNoLock(); - std::string objectStorePath = be->getParams()->toURL(); + auto params = be->getParams(); + [[maybe_unused]] std::unique_ptr paramsCleanupPtr(params); // Ensures the params pointer is always cleaned up correctly + std::string objectStorePath = params->toURL(); if (dump_object_body_only) { cta::utils::json::object::JSONCObject jObject; jObject.buildFromJSON(ge.dump().second); @@ -120,7 +122,7 @@ int main(int argc, char ** argv) { jObject.buildFromJSON(oss.str()); std::cout << jObject.getJSONPretty() << std::endl; } else { - std::cout << "Object store path: " << be->getParams()->toURL() << std::endl; + std::cout << "Object store path: " << params->toURL() << std::endl; std::cout << "Object name: " << objectName << std::endl; std::cout << "Header dump:" << std::endl; std::cout << headerDump; diff --git a/objectstore/cta-objectstore-initialize.cpp b/objectstore/cta-objectstore-initialize.cpp index 0c9a57b3e1..260504048f 100644 --- a/objectstore/cta-objectstore-initialize.cpp +++ b/objectstore/cta-objectstore-initialize.cpp @@ -71,7 +71,9 @@ int main(int argc, char ** argv) { ag.removeAndUnregisterSelf(lc); } rel.release(); - std::cout << "New object store path: " << be->getParams()->toURL() << std::endl; + auto params = be->getParams(); + [[maybe_unused]] std::unique_ptr paramsCleanupPtr(params); // Ensures the params pointer is always cleaned up correctly + std::cout << "New object store path: " << params->toURL() << std::endl; return EXIT_SUCCESS; } catch (std::exception & e) { std::cerr << "Failed to initialise the root entry in a new " << ((be != nullptr) ? be->typeName() : "no-backend") << " objectstore" diff --git a/objectstore/cta-objectstore-list.cpp b/objectstore/cta-objectstore-list.cpp index 5b2e015734..7125a4e022 100644 --- a/objectstore/cta-objectstore-list.cpp +++ b/objectstore/cta-objectstore-list.cpp @@ -50,7 +50,9 @@ int main(int argc, char ** argv) { dynamic_cast(*be).noDeleteOnExit(); } catch (std::bad_cast &){} // If not, nevermind. - std::cout << "Object store path: " << be->getParams()->toURL() << std::endl; + auto params = be->getParams(); + [[maybe_unused]] std::unique_ptr paramsCleanupPtr(params); // Ensures the params pointer is always cleaned up correctly + std::cout << "Object store path: " << params->toURL() << std::endl; auto l = be->list(); for (auto o=l.begin(); o!=l.end(); o++) { std::cout << *o << std::endl; -- GitLab From 0d03414ba320667a094bab59ec4783bf28737c8d Mon Sep 17 00:00:00 2001 From: Niels Alexander Buegel Date: Thu, 27 Nov 2025 00:03:58 +0100 Subject: [PATCH 2/3] Buffer overflow fixed in rmcd --- mediachanger/librmc/Cdomainname.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/mediachanger/librmc/Cdomainname.cpp b/mediachanger/librmc/Cdomainname.cpp index d69d8f2c3c..639c4bb471 100644 --- a/mediachanger/librmc/Cdomainname.cpp +++ b/mediachanger/librmc/Cdomainname.cpp @@ -51,9 +51,14 @@ int Cdomainname(char* name, int namelen) { if (*p != '\0') { *(p + strlen(p) - 1) = '\0'; } - name[namelen] = '\0'; - strncpy(name, p, namelen + 1); - if (name[namelen] != '\0') { + if (namelen == 0) { + serrno = EINVAL; + return -1; + } + size_t maxcopy = namelen - 1; + name[maxcopy] = '\0'; + strncpy(name, p, maxcopy); + if (name[maxcopy] != '\0') { serrno = EINVAL; return -1; } -- GitLab From be1fc72c16a5fe4905a70a5f6d310531c5aeaf8a Mon Sep 17 00:00:00 2001 From: Niels Alexander Buegel Date: Wed, 3 Dec 2025 11:56:16 +0100 Subject: [PATCH 3/3] Better memory leak fixes --- continuousintegration/build_deploy.sh | 2 +- objectstore/Backend.hpp | 3 ++- objectstore/BackendRados.cpp | 6 +++--- objectstore/BackendRados.hpp | 2 +- objectstore/BackendVFS.cpp | 6 +++--- objectstore/BackendVFS.hpp | 2 +- objectstore/cta-objectstore-collect-orphaned-object.cpp | 1 - objectstore/cta-objectstore-create-missing-repack-index.cpp | 1 - objectstore/cta-objectstore-dereference-removed-queues.cpp | 1 - objectstore/cta-objectstore-dump-object.cpp | 1 - objectstore/cta-objectstore-initialize.cpp | 1 - objectstore/cta-objectstore-list.cpp | 1 - 12 files changed, 11 insertions(+), 16 deletions(-) diff --git a/continuousintegration/build_deploy.sh b/continuousintegration/build_deploy.sh index 973f80e33c..ed96de72f4 100755 --- a/continuousintegration/build_deploy.sh +++ b/continuousintegration/build_deploy.sh @@ -370,7 +370,7 @@ build_deploy() { fi if [[ ${enable_address_sanitizer} = true ]]; then - build_rpm_flags+=" --address-sanitizer" + build_rpm_flags+=" --enable-address-sanitizer" fi print_header "BUILDING RPMS" diff --git a/objectstore/Backend.hpp b/objectstore/Backend.hpp index ccd6928619..8c6a89871c 100644 --- a/objectstore/Backend.hpp +++ b/objectstore/Backend.hpp @@ -20,6 +20,7 @@ #include #include #include +#include #include "common/exception/Exception.hpp" @@ -246,7 +247,7 @@ class Backend { * Returns a type specific representation of the parameters * @return pointer to the newly created representation. */ - virtual Parameters * getParams() = 0; + virtual std::unique_ptr getParams() = 0; /** * Return the name of the class. Mostly usefull in tests diff --git a/objectstore/BackendRados.cpp b/objectstore/BackendRados.cpp index a87fb63596..11dd1685a0 100644 --- a/objectstore/BackendRados.cpp +++ b/objectstore/BackendRados.cpp @@ -1087,12 +1087,12 @@ std::string BackendRados::Parameters::toURL() { } -BackendRados::Parameters* BackendRados::getParams() { - std::unique_ptr ret(new Parameters); +std::unique_ptr BackendRados::getParams() { + auto ret = std::make_unique(); ret->m_pool = m_pool; ret->m_userId = m_user; ret->m_namespace = m_namespace; - return ret.release(); + return ret; } } // namespace cta::objectstore diff --git a/objectstore/BackendRados.hpp b/objectstore/BackendRados.hpp index f4fb125859..df7dbb8a51 100644 --- a/objectstore/BackendRados.hpp +++ b/objectstore/BackendRados.hpp @@ -347,7 +347,7 @@ public: std::string m_namespace; }; - Parameters * getParams() override; + std::unique_ptr getParams() override; std::string typeName() override { return "cta::objectstore::BackendRados"; diff --git a/objectstore/BackendVFS.cpp b/objectstore/BackendVFS.cpp index 922b772fe3..d567ee16ea 100644 --- a/objectstore/BackendVFS.cpp +++ b/objectstore/BackendVFS.cpp @@ -228,10 +228,10 @@ std::list BackendVFS::list() { return ret; } -BackendVFS::Parameters* BackendVFS::getParams() { - std::unique_ptr ret(new Parameters); +std::unique_ptr BackendVFS::getParams() { + auto ret = std::make_unique(); ret->m_path = m_root; - return ret.release(); + return ret; } void BackendVFS::ScopedLock::release() { diff --git a/objectstore/BackendVFS.hpp b/objectstore/BackendVFS.hpp index f600ac0c81..907c3d23fc 100644 --- a/objectstore/BackendVFS.hpp +++ b/objectstore/BackendVFS.hpp @@ -200,7 +200,7 @@ class BackendVFS: public Backend { std::string m_path; }; - Parameters * getParams() override; + std::unique_ptr getParams() override; std::string typeName() override { diff --git a/objectstore/cta-objectstore-collect-orphaned-object.cpp b/objectstore/cta-objectstore-collect-orphaned-object.cpp index 60335aa6a7..761a599ec9 100644 --- a/objectstore/cta-objectstore-collect-orphaned-object.cpp +++ b/objectstore/cta-objectstore-collect-orphaned-object.cpp @@ -78,7 +78,6 @@ int main(int argc, char ** argv) { dynamic_cast(*be).noDeleteOnExit(); } catch (std::bad_cast &){} auto params = be->getParams(); - [[maybe_unused]] std::unique_ptr paramsCleanupPtr(params); // Ensures the params pointer is always cleaned up correctly std::cout << "Object store path: " << params->toURL() << std::endl; // Try and open the object. cta::objectstore::GenericObject go(objectName, *be); diff --git a/objectstore/cta-objectstore-create-missing-repack-index.cpp b/objectstore/cta-objectstore-create-missing-repack-index.cpp index 19fdb3f318..75d28bf988 100644 --- a/objectstore/cta-objectstore-create-missing-repack-index.cpp +++ b/objectstore/cta-objectstore-create-missing-repack-index.cpp @@ -56,7 +56,6 @@ int main(int argc, char ** argv) { dynamic_cast(*be).noDeleteOnExit(); } catch (std::bad_cast &){} auto params = be->getParams(); - [[maybe_unused]] std::unique_ptr paramsCleanupPtr(params); // Ensures the params pointer is always cleaned up correctly std::cout << "Object store path: " << params->toURL() << std::endl; // Open the root entry RW std::cout << "Creating AgentReference for the creation of the repack index" << std::endl; diff --git a/objectstore/cta-objectstore-dereference-removed-queues.cpp b/objectstore/cta-objectstore-dereference-removed-queues.cpp index b62f2ac6f0..5853ef4163 100644 --- a/objectstore/cta-objectstore-dereference-removed-queues.cpp +++ b/objectstore/cta-objectstore-dereference-removed-queues.cpp @@ -103,7 +103,6 @@ int main(int argc, char ** argv) { dynamic_cast(*be).noDeleteOnExit(); } catch (std::bad_cast &){} auto params = be->getParams(); - [[maybe_unused]] std::unique_ptr paramsCleanupPtr(params); // Ensures the params pointer is always cleaned up correctly std::cout << "Object store path: " << params->toURL() << std::endl; // Open the root entry RW cta::objectstore::RootEntry re(*be); diff --git a/objectstore/cta-objectstore-dump-object.cpp b/objectstore/cta-objectstore-dump-object.cpp index 722ad3bb7e..eb0717b001 100644 --- a/objectstore/cta-objectstore-dump-object.cpp +++ b/objectstore/cta-objectstore-dump-object.cpp @@ -103,7 +103,6 @@ int main(int argc, char ** argv) { cta::objectstore::GenericObject ge(objectName, *be); ge.fetchNoLock(); auto params = be->getParams(); - [[maybe_unused]] std::unique_ptr paramsCleanupPtr(params); // Ensures the params pointer is always cleaned up correctly std::string objectStorePath = params->toURL(); if (dump_object_body_only) { cta::utils::json::object::JSONCObject jObject; diff --git a/objectstore/cta-objectstore-initialize.cpp b/objectstore/cta-objectstore-initialize.cpp index 260504048f..d9bf9ec54a 100644 --- a/objectstore/cta-objectstore-initialize.cpp +++ b/objectstore/cta-objectstore-initialize.cpp @@ -72,7 +72,6 @@ int main(int argc, char ** argv) { } rel.release(); auto params = be->getParams(); - [[maybe_unused]] std::unique_ptr paramsCleanupPtr(params); // Ensures the params pointer is always cleaned up correctly std::cout << "New object store path: " << params->toURL() << std::endl; return EXIT_SUCCESS; } catch (std::exception & e) { diff --git a/objectstore/cta-objectstore-list.cpp b/objectstore/cta-objectstore-list.cpp index 7125a4e022..67006a8a02 100644 --- a/objectstore/cta-objectstore-list.cpp +++ b/objectstore/cta-objectstore-list.cpp @@ -51,7 +51,6 @@ int main(int argc, char ** argv) { } catch (std::bad_cast &){} // If not, nevermind. auto params = be->getParams(); - [[maybe_unused]] std::unique_ptr paramsCleanupPtr(params); // Ensures the params pointer is always cleaned up correctly std::cout << "Object store path: " << params->toURL() << std::endl; auto l = be->list(); for (auto o=l.begin(); o!=l.end(); o++) { -- GitLab