From 9929f2cacfe86286e01a9c09b7db388b10de68a4 Mon Sep 17 00:00:00 2001 From: ankor2023 Date: Wed, 9 Jul 2025 17:45:19 +0000 Subject: [PATCH] Reduce UDS/segment name clashes across same-service instances (#2023) Add a PID file name hash to the names of the shared memory segments and Unix Domain Sockets. Since all instances running on the same host are supposed to have unique PID files, this addition significantly reduces the probability of name clashes when running multiple Squid instances with the same service name (i.e. the same `squid -n` parameter value that defaults to "squid"). A clash may still happen if two different PID file names have the same hash or if multiple instances disable PID file management with `pid_filename none`. Clashes may also happen in environments where Squid does not even use service name for naming shared memory segments. Examples of UDS and shared memory segment names (while using default service name): /var/run/squid/squid-SLWQ-kid-1.ipc /var/run/squid/squid-SLWQ-coordinator.ipc /dev/shm/squid-SLWQ-tls_session_cache.shm /dev/shm/squid-SLWQ-transients_map_slices.shm This change is a reference point for automated CONTRIBUTORS updates. --- CONTRIBUTORS | 2 +- src/Instance.cc | 41 ++++++++++++++++++++++++++++++++++++++ src/Instance.h | 10 ++++++++++ src/Makefile.am | 8 ++++++++ src/ipc/Port.cc | 11 +++++----- src/ipc/mem/Segment.cc | 6 +++--- src/tests/Stub.am | 1 + src/tests/stub_Instance.cc | 20 +++++++++++++++++++ 8 files changed, 89 insertions(+), 10 deletions(-) create mode 100644 src/tests/stub_Instance.cc diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 5ac59a8294..2bc0a1e597 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -55,7 +55,7 @@ Thank you! Andrew Tridgell Andrey Andrey Shorin - ankor2023 <138755079+ankor2023@users.noreply.github.com> + Ankor Anonymous Anonymous Anonymous Pootle User diff --git a/src/Instance.cc b/src/Instance.cc index 1f3b10cc69..3460a95021 100644 --- a/src/Instance.cc +++ b/src/Instance.cc @@ -11,6 +11,7 @@ #include "debug/Messages.h" #include "fs_io.h" #include "Instance.h" +#include "md5.h" #include "parser/Tokenizer.h" #include "sbuf/Stream.h" #include "SquidConfig.h" @@ -222,3 +223,43 @@ Instance::WriteOurPid() debugs(50, Important(23), "Created " << TheFile); } +/// A hash that is likely to be unique across instances running on the same host +/// because such concurrent instances should use unique PID filenames. +/// All instances with disabled PID file maintenance have the same hash value. +/// \returns a 4-character string suitable for use in file names and similar contexts +static SBuf +PidFilenameHash() +{ + uint8_t hash[SQUID_MD5_DIGEST_LENGTH]; + + SquidMD5_CTX ctx; + SquidMD5Init(&ctx); + const auto name = PidFilenameCalc(); + SquidMD5Update(&ctx, name.rawContent(), name.length()); + SquidMD5Final(hash, &ctx); + + // converts raw hash byte at a given position to a filename-suitable character + const auto hashAt = [&hash](const size_t idx) { + const auto safeChars = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; + return safeChars[hash[idx] % strlen(safeChars)]; + }; + + SBuf buf; + buf.appendf("%c%c%c%c", hashAt(0), hashAt(1), hashAt(2), hashAt(3)); + return buf; +} + +SBuf +Instance::NamePrefix(const char * const head, const char * const tail) +{ + SBuf buf(head); + buf.append(service_name); + buf.append("-"); + buf.append(PidFilenameHash()); + if (tail) { + // TODO: Remove leading "-" from callers and explicitly add it here. + buf.append(tail); + } + return buf; +} + diff --git a/src/Instance.h b/src/Instance.h index b0e8bdff3a..2a83abaaaf 100644 --- a/src/Instance.h +++ b/src/Instance.h @@ -9,6 +9,8 @@ #ifndef SQUID_SRC_INSTANCE_H #define SQUID_SRC_INSTANCE_H +#include "sbuf/forward.h" + #if HAVE_SYS_TYPES_H #include #endif @@ -30,6 +32,14 @@ void WriteOurPid(); /// Throws if PID file maintenance is disabled. pid_t Other(); +/// A service_name-derived string that is likely to be unique across all Squid +/// instances concurrently running on the same host (as long as they do not +/// disable PID file maintenance). +/// \param head is used at the beginning of the generated name +/// \param tail is used at the end of the generated name (when not nil) +/// \returns a head-...tail string suitable for making file and shm segment names +SBuf NamePrefix(const char *head, const char *tail = nullptr); + } // namespace Instance #endif /* SQUID_SRC_INSTANCE_H */ diff --git a/src/Makefile.am b/src/Makefile.am index 5d06101945..ac2586fd92 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1033,6 +1033,7 @@ tests_testRock_SOURCES = \ HttpHeaderTools.h \ HttpReply.cc \ tests/stub_HttpRequest.cc \ + tests/stub_Instance.cc \ LogTags.cc \ MasterXaction.cc \ MasterXaction.h \ @@ -1203,6 +1204,7 @@ tests_testUfs_SOURCES = \ HttpHeaderTools.h \ HttpReply.cc \ tests/stub_HttpRequest.cc \ + tests/stub_Instance.cc \ LogTags.cc \ MasterXaction.cc \ MasterXaction.h \ @@ -1375,6 +1377,7 @@ tests_testStore_SOURCES = \ HttpHeaderTools.h \ tests/stub_HttpReply.cc \ tests/stub_HttpRequest.cc \ + tests/stub_Instance.cc \ MasterXaction.cc \ MasterXaction.h \ MemBuf.cc \ @@ -1538,6 +1541,7 @@ tests_testDiskIO_SOURCES = \ HttpHeaderTools.h \ HttpReply.cc \ tests/stub_HttpRequest.cc \ + tests/stub_Instance.cc \ LogTags.cc \ MasterXaction.cc \ MasterXaction.h \ @@ -1801,6 +1805,7 @@ tests_testHttpRange_SOURCES = \ HttpReply.cc \ HttpRequest.cc \ tests/stub_HttpUpgradeProtocolAccess.cc \ + tests/stub_Instance.cc \ IoStats.h \ tests/stub_IpcIoFile.cc \ LogTags.cc \ @@ -2061,6 +2066,7 @@ tests_testHttpReply_SOURCES = \ tests/testHttpReply.cc \ HttpReply.h \ tests/stub_HttpRequest.cc \ + tests/stub_Instance.cc \ MasterXaction.cc \ MasterXaction.h \ MemBuf.cc \ @@ -2188,6 +2194,7 @@ tests_testHttpRequest_SOURCES = \ tests/testHttpRequest.cc \ tests/testHttpRequestMethod.cc \ tests/stub_HttpUpgradeProtocolAccess.cc \ + tests/stub_Instance.cc \ IoStats.h \ tests/stub_IpcIoFile.cc \ LogTags.cc \ @@ -2485,6 +2492,7 @@ tests_testCacheManager_SOURCES = \ HttpReply.cc \ HttpRequest.cc \ tests/stub_HttpUpgradeProtocolAccess.cc \ + tests/stub_Instance.cc \ IoStats.h \ tests/stub_IpcIoFile.cc \ LogTags.cc \ diff --git a/src/ipc/Port.cc b/src/ipc/Port.cc index 2446d1bb7a..ddf2e5881b 100644 --- a/src/ipc/Port.cc +++ b/src/ipc/Port.cc @@ -13,8 +13,10 @@ #include "comm/Connection.h" #include "comm/Read.h" #include "CommCalls.h" +#include "Instance.h" #include "ipc/Port.h" #include "sbuf/Stream.h" +#include "sbuf/StringConvert.h" #include "tools.h" #include "util.h" @@ -52,9 +54,8 @@ bool Ipc::Port::doneAll() const String Ipc::Port::MakeAddr(const char* processLabel, int id) { assert(id >= 0); - String addr = channelPathPfx; - addr.append(service_name.c_str()); - addr.append(processLabel); + String addr; + addr.append(SBufToString(Instance::NamePrefix(channelPathPfx, processLabel))); addr.append('-'); addr.append(xitoa(id)); addr.append(".ipc"); @@ -66,9 +67,7 @@ Ipc::Port::CoordinatorAddr() { static String coordinatorAddr; if (!coordinatorAddr.size()) { - coordinatorAddr= channelPathPfx; - coordinatorAddr.append(service_name.c_str()); - coordinatorAddr.append(coordinatorAddrLabel); + coordinatorAddr.append(SBufToString(Instance::NamePrefix(channelPathPfx, coordinatorAddrLabel))); coordinatorAddr.append(".ipc"); } return coordinatorAddr; diff --git a/src/ipc/mem/Segment.cc b/src/ipc/mem/Segment.cc index 67ba4431f1..40ea28e388 100644 --- a/src/ipc/mem/Segment.cc +++ b/src/ipc/mem/Segment.cc @@ -13,8 +13,9 @@ #include "compat/shm.h" #include "debug/Stream.h" #include "fatal.h" +#include "Instance.h" #include "ipc/mem/Segment.h" -#include "sbuf/SBuf.h" +#include "sbuf/StringConvert.h" #include "SquidConfig.h" #include "tools.h" @@ -277,8 +278,7 @@ Ipc::Mem::Segment::GenerateName(const char *id) if (name[name.size()-1] != '/') name.append('/'); } else { - name.append('/'); - name.append(service_name.c_str()); + name.append(SBufToString(Instance::NamePrefix("/"))); name.append('-'); } diff --git a/src/tests/Stub.am b/src/tests/Stub.am index ad03078aba..2f9d15d279 100644 --- a/src/tests/Stub.am +++ b/src/tests/Stub.am @@ -22,6 +22,7 @@ STUB_SOURCE = \ tests/stub_HttpReply.cc \ tests/stub_HttpRequest.cc \ tests/stub_HttpUpgradeProtocolAccess.cc \ + tests/stub_Instance.cc \ tests/stub_IpcIoFile.cc \ tests/stub_MemBuf.cc \ tests/stub_MemObject.cc \ diff --git a/src/tests/stub_Instance.cc b/src/tests/stub_Instance.cc new file mode 100644 index 0000000000..72fa443065 --- /dev/null +++ b/src/tests/stub_Instance.cc @@ -0,0 +1,20 @@ +/* + * Copyright (C) 1996-2025 The Squid Software Foundation and contributors + * + * Squid software is distributed under GPLv2+ license and includes + * contributions from numerous individuals and organizations. + * Please see the COPYING and CONTRIBUTORS files for details. + */ + +#include "squid.h" +#include "Instance.h" +#include "sbuf/SBuf.h" + +#define STUB_API "Instance.cc" +#include "tests/STUB.h" + +void Instance::ThrowIfAlreadyRunning() STUB +void Instance::WriteOurPid() STUB +pid_t Instance::Other() STUB_RETVAL({}) +SBuf Instance::NamePrefix(const char *, const char *) STUB_RETVAL_NOP(SBuf("squid-0")) + -- 2.47.3