From a472992900346a3b621fa41a8a65e7da3486e2b9 Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Mon, 13 Jan 2025 14:21:35 +0100 Subject: [PATCH 1/2] rec: delete temp file on failure to dump RPZ file --- pdns/recursordist/rpzloader.cc | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/pdns/recursordist/rpzloader.cc b/pdns/recursordist/rpzloader.cc index 1ba0c9b6564d..297fe490e5c8 100644 --- a/pdns/recursordist/rpzloader.cc +++ b/pdns/recursordist/rpzloader.cc @@ -379,6 +379,18 @@ std::shared_ptr loadRPZFromFile(const std::string& fname return soaRecordContent; } +struct FilenameDeleter +{ + void operator()(const string* name) const noexcept + { + if (!name->empty()) { + unlink(name->c_str()); + } + } +}; + +using UniqueFilenameDeleterPtr = std::unique_ptr; + static bool dumpZoneToDisk(Logr::log_t logger, const std::shared_ptr& newZone, const std::string& dumpZoneFileName) { logger->info(Logr::Debug, "Dumping zone to disk", "destination_file", Logging::Loggable(dumpZoneFileName)); @@ -395,11 +407,12 @@ static bool dumpZoneToDisk(Logr::log_t logger, const std::shared_ptrgetDomain() != soa.d_name) { logger->info(Logr::Error, "Inconsistency of internal name and SOA name", "zone", Logging::Loggable(newZone->getDomain()), "soaname", Logging::Loggable(soa.d_name)); } - std::string temp = dumpZoneFileName + "XXXXXX"; - int fileDesc = mkstemp(&temp.at(0)); + auto tempFile = UniqueFilenameDeleterPtr(new string(dumpZoneFileName + "XXXXXX")); + int fileDesc = mkstemp(tempFile->data()); if (fileDesc < 0) { SLOG(g_log << Logger::Warning << "Unable to open a file to dump the content of the RPZ zone " << zoneName << endl, logger->error(Logr::Error, errno, "Unable to create temporary file")); + tempFile->clear(); // file has not been created, no need to unlink return false; } @@ -439,12 +452,12 @@ static bool dumpZoneToDisk(Logr::log_t logger, const std::shared_ptrc_str(), dumpZoneFileName.c_str()) != 0) { SLOG(g_log << Logger::Warning << "Error while moving the content of the RPZ zone " << zoneName << " to the dump file: " << stringerror() << endl, logger->error(Logr::Error, errno, "Error while moving the content of the RPZ", "destination_file", Logging::Loggable(dumpZoneFileName))); return false; } - + tempFile->clear(); // file has been renamed, no need to unlink return true; } From 9817dc5c7b570ee6d81e2e5cf6855b1a96a8efb3 Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Mon, 13 Jan 2025 15:45:00 +0100 Subject: [PATCH 2/2] Don't forget to delete the pointer itself --- pdns/recursordist/rpzloader.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pdns/recursordist/rpzloader.cc b/pdns/recursordist/rpzloader.cc index 297fe490e5c8..720340e103d5 100644 --- a/pdns/recursordist/rpzloader.cc +++ b/pdns/recursordist/rpzloader.cc @@ -383,8 +383,11 @@ struct FilenameDeleter { void operator()(const string* name) const noexcept { - if (!name->empty()) { - unlink(name->c_str()); + if (name != nullptr) { + if (!name->empty()) { + unlink(name->c_str()); + } + delete name; // NOLINT(cppcoreguidelines-owning-memory) } } };