From f56930f2e753d22924c2402a0ec049242129b81a Mon Sep 17 00:00:00 2001 From: bneradt Date: Tue, 11 Aug 2026 13:40:56 -0500 Subject: [PATCH] cache: fix shm sizing on large-page Linux POSIX shared-memory objects on Linux retain the exact length passed to ftruncate(), but the cache shm gates accepted any size through the next page boundary. On 64 KiB-page systems, a foreign control layout could therefore be treated as compatible, causing cleanup and tooling paths to walk an untrusted stripe table and leave segments behind. This patch requires exact shared-memory object sizes outside macOS while preserving macOS's page-rounded allowance. It keeps the foreign-layout test at its original size and directly covers the platform-specific sizing contract. Fixes: #13534 --- include/shared/cache_shm/Layout.h | 21 ++++++++++++++++---- include/shared/cache_shm/Purge.h | 7 +++---- src/iocore/cache/CacheShm.cc | 8 ++------ src/iocore/cache/unit_tests/test_CacheShm.cc | 17 ++++++++++++++-- 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/include/shared/cache_shm/Layout.h b/include/shared/cache_shm/Layout.h index fa6a8e65228..be370a346a6 100644 --- a/include/shared/cache_shm/Layout.h +++ b/include/shared/cache_shm/Layout.h @@ -87,13 +87,26 @@ constexpr std::size_t CONTROL_HEADER_SIZE = offsetof(CacheShmControl, stripes); static_assert(CONTROL_HEADER_SIZE == 48, "the control segment header is a frozen layout; see the comment above"); static_assert(std::is_standard_layout_v, "the control segment is shared across processes and builds"); -// Whether a control segment of `actual` bytes was written by *this* build; the kernel rounds an shm object up to a page. -// Anything larger has a stripes[] of unknown stride and must never be walked with our layout. Shared by the attach gate, -// the purge primitive and `traffic_ctl cache shm status` so the three cannot drift apart. +/// Whether @a actual is the object size this platform reports after truncating +/// a POSIX shared-memory object to @a requested bytes. +inline bool +is_expected_shm_size(std::size_t actual, std::size_t requested) +{ +#if defined(__APPLE__) + // macOS rounds the reported object size up to the VM page size. + return actual >= requested && actual <= INK_ALIGN(requested, ats_pagesize()); +#else + return actual == requested; +#endif +} + +// Whether a control segment of `actual` bytes was written by *this* build. Anything else has a stripes[] of unknown +// stride and must never be walked with our layout. Shared by the attach gate, the purge primitive and +// `traffic_ctl cache shm status` so the three cannot drift apart. inline bool is_own_control_size(std::size_t actual) { - return actual >= CONTROL_SIZE && actual <= INK_ALIGN(CONTROL_SIZE, ats_pagesize()); + return is_expected_shm_size(actual, CONTROL_SIZE); } // Frame the operator's middle word (e.g. "ats") as "/-". The framing is supplied here so it cannot be mis-typed: diff --git a/include/shared/cache_shm/Purge.h b/include/shared/cache_shm/Purge.h index 1b170b385e1..39ff9966bcb 100644 --- a/include/shared/cache_shm/Purge.h +++ b/include/shared/cache_shm/Purge.h @@ -286,10 +286,9 @@ purge_segments(const std::string &prefix) return report; } - // Larger than this build's page-rounded CONTROL_SIZE means a build with a different - // sizeof(CacheShmControl) wrote it. The frozen header prefix is still readable (so - // the owner guard above applies), but stripes[] may have a different stride entirely, - // so its names must not drive shm_unlink. + // A control size this build does not accept means a build with a different sizeof(CacheShmControl) wrote it. The frozen + // header prefix is still readable (so the owner guard above applies), but stripes[] may have a different stride + // entirely, so its names must not drive shm_unlink. if (magic_ok && is_own_control_size(static_cast(sb.st_size))) { unlink_table_stripes(prefix, ctrl, report.unlinked); } else { diff --git a/src/iocore/cache/CacheShm.cc b/src/iocore/cache/CacheShm.cc index 157a5097b54..69569822b79 100644 --- a/src/iocore/cache/CacheShm.cc +++ b/src/iocore/cache/CacheShm.cc @@ -270,14 +270,10 @@ open_and_map_shm(const std::string &name, std::size_t size, ShmAccess access, [[ return nullptr; } } else { - // The kernel rounds an shm object up to a page, so accept any size in [requested, page-up]. struct stat sb { }; - std::size_t expected_max = INK_ALIGN(size, ats_pagesize()); - if (fstat(fd, &sb) < 0 || sb.st_size < 0 || static_cast(sb.st_size) < size || - static_cast(sb.st_size) > expected_max) { - Dbg(dbg_ctl, "shm %s size mismatch (have %lld, want %zu, max %zu)", name.c_str(), static_cast(sb.st_size), size, - expected_max); + if (fstat(fd, &sb) < 0 || sb.st_size < 0 || !cache_shm::is_expected_shm_size(static_cast(sb.st_size), size)) { + Dbg(dbg_ctl, "shm %s size mismatch (have %lld, want %zu)", name.c_str(), static_cast(sb.st_size), size); return nullptr; } } diff --git a/src/iocore/cache/unit_tests/test_CacheShm.cc b/src/iocore/cache/unit_tests/test_CacheShm.cc index 790e4841845..87380133bf0 100644 --- a/src/iocore/cache/unit_tests/test_CacheShm.cc +++ b/src/iocore/cache/unit_tests/test_CacheShm.cc @@ -225,6 +225,19 @@ TEST_CASE("CacheShm process liveness check backs the concurrent-attach guard", " CHECK_FALSE(CacheShm::process_is_alive(std::numeric_limits::max())); } +TEST_CASE("CacheShm object size matching follows platform behavior", "[cache][shm]") +{ + constexpr std::size_t requested = cache_shm::CONTROL_SIZE; + + CHECK(cache_shm::is_expected_shm_size(requested, requested)); + CHECK_FALSE(cache_shm::is_expected_shm_size(requested - 1, requested)); +#if defined(__APPLE__) + CHECK(cache_shm::is_expected_shm_size(INK_ALIGN(requested, ats_pagesize()), requested)); +#else + CHECK_FALSE(cache_shm::is_expected_shm_size(requested + sizeof(cache_shm::StripeEntry), requested)); +#endif +} + // The rest of this file needs real shm objects, unlike the layout/fingerprint cases // above, so it is gated the same way the feature is. #if TS_USE_CACHE_SHM @@ -293,8 +306,8 @@ segment_exists(const std::string &name) return true; } -// The kernel rounds an shm object up to a page, so a segment shorter than CONTROL_SIZE is not representable everywhere: -// Apple Silicon's 16 KB page already exceeds it. -1 if the segment is gone. +// macOS rounds a POSIX shm object up to a page, so a segment shorter than CONTROL_SIZE is not representable there. +// -1 if the segment is gone. long long segment_size(const std::string &name) {