From 7d53bbc90d7f6381a0ca69c23a4f8fd73430913e Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Thu, 27 Aug 2026 06:30:03 +0000 Subject: [PATCH 01/15] Harden against invalid cache peer digests An assert() call was improperly used to validate a received cache digest from a cache_peer. Switch to using an invalid value, so that the improper digest is rejected instead. --- src/CacheDigest.cc | 7 +++++-- src/CacheDigest.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/CacheDigest.cc b/src/CacheDigest.cc index f5b1d985114..ecc88c2284e 100644 --- a/src/CacheDigest.cc +++ b/src/CacheDigest.cc @@ -14,6 +14,8 @@ #include "Store.h" #include "store_key_md5.h" +#include + #if USE_CACHE_DIGESTS #include "CacheDigest.h" @@ -270,8 +272,9 @@ cacheDigestReport(CacheDigest * cd, const SBuf &label, StoreEntry * e) uint32_t CacheDigest::CalcMaskSize(uint64_t cap, uint8_t bpe) { - uint64_t bitCount = (cap * bpe) + 7; - assert(bitCount < INT_MAX); // do not 31-bit overflow later + const uint64_t bitCount = (cap * bpe) + 7; + if (bitCount >= std::numeric_limits::max()) + return 0; // overflow; caller must treat 0 as invalid return static_cast(bitCount / 8); } diff --git a/src/CacheDigest.h b/src/CacheDigest.h index 2b49ed3d435..7ad45b48fe3 100644 --- a/src/CacheDigest.h +++ b/src/CacheDigest.h @@ -45,6 +45,7 @@ class CacheDigest /// calculate the size of mask required to digest up to /// a specified capacity and bitsize. + /// \returns 0 when inputs would overflow (invalid). static uint32_t CalcMaskSize(uint64_t cap, uint8_t bpe); private: From 077262eb99c9f89b29f81d8ab4486786f9dea420 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Tue, 1 Sep 2026 12:30:43 -0400 Subject: [PATCH 02/15] fixup: Do not return compiler-valid zero on invalid input Also marked a few unaddressed problems. Addressing them may change this solution! --- src/CacheDigest.cc | 18 +++++++++++------- src/CacheDigest.h | 6 ++++-- src/peer_digest.cc | 19 +++++++++++++++---- src/tests/stub_CacheDigest.cc | 2 +- 4 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/CacheDigest.cc b/src/CacheDigest.cc index ecc88c2284e..518244c3399 100644 --- a/src/CacheDigest.cc +++ b/src/CacheDigest.cc @@ -39,11 +39,15 @@ static uint32_t hashed_keys[4]; void CacheDigest::init(uint64_t newCapacity) { - const auto newMaskSz = CacheDigest::CalcMaskSize(newCapacity, bits_per_entry); - assert(newCapacity > 0 && bits_per_entry > 0); - assert(newMaskSz != 0); + assert(newCapacity > 0); capacity = newCapacity; - mask_size = newMaskSz; + + assert(bits_per_entry > 0); + const auto newMaskSz = CacheDigest::CalcMaskSize(newCapacity, bits_per_entry); + assert(newMaskSz); // assume that newCapacity and bits_per_entry have been validated w.r.t. mask size overflows (XXX?) + mask_size = *newMaskSz; + assert(mask_size > 0); + mask = static_cast(xcalloc(mask_size,1)); debugs(70, 2, "capacity: " << capacity << " entries, bpe: " << bits_per_entry << "; size: " << mask_size << " bytes"); @@ -269,12 +273,12 @@ cacheDigestReport(CacheDigest * cd, const SBuf &label, StoreEntry * e) ); } -uint32_t +std::optional CacheDigest::CalcMaskSize(uint64_t cap, uint8_t bpe) { - const uint64_t bitCount = (cap * bpe) + 7; + const uint64_t bitCount = (cap * bpe) + 7; // XXX: Overflows! if (bitCount >= std::numeric_limits::max()) - return 0; // overflow; caller must treat 0 as invalid + return std::nullopt; // overflow return static_cast(bitCount / 8); } diff --git a/src/CacheDigest.h b/src/CacheDigest.h index 7ad45b48fe3..5e976d41e38 100644 --- a/src/CacheDigest.h +++ b/src/CacheDigest.h @@ -14,6 +14,8 @@ #include "mem/forward.h" #include "store_key_md5.h" +#include + class CacheDigestGuessStats; class StoreEntry; @@ -45,8 +47,8 @@ class CacheDigest /// calculate the size of mask required to digest up to /// a specified capacity and bitsize. - /// \returns 0 when inputs would overflow (invalid). - static uint32_t CalcMaskSize(uint64_t cap, uint8_t bpe); + /// \returns nil on overflows (i.e. when our mask_size would not be able to safely store the computed mask size) + static std::optional CalcMaskSize(uint64_t cap, uint8_t bpe); private: void init(uint64_t newCapacity); diff --git a/src/peer_digest.cc b/src/peer_digest.cc index 8660242d0fb..57a46c6d7fd 100644 --- a/src/peer_digest.cc +++ b/src/peer_digest.cc @@ -29,6 +29,8 @@ #include "tools.h" #include "util.h" +#include + /* local types */ /* local prototypes */ @@ -779,11 +781,20 @@ peerDigestSetCBlock(PeerDigest * pd, const char *buf) return 0; } - /* check consistency further */ - if ((size_t)cblock.mask_size != CacheDigest::CalcMaskSize(cblock.capacity, cblock.bits_per_entry)) { + const auto maskSize = CacheDigest::CalcMaskSize(cblock.capacity, cblock.bits_per_entry); + if (!maskSize) { + // if we cannot compute maskSize, then received cblock.mask_size cannot hold that value either + using ComputedMaskSizeType = std::remove_cv_t >; + static_assert(std::numeric_limits::max() >= std::numeric_limits::max()); + debugs(72, DBG_CRITICAL, host << " digest cblock is corrupted " << + "(mask size too small: " << cblock.mask_size << " bytes for " << + cblock.capacity << " entries with " << cblock.bits_per_entry << " bpe)."); + return 0; + } + + if ((size_t)cblock.mask_size != *maskSize) { debugs(72, DBG_CRITICAL, host << " digest cblock is corrupted " << - "(mask size mismatch: " << cblock.mask_size << " ? " << - CacheDigest::CalcMaskSize(cblock.capacity, cblock.bits_per_entry) + "(mask size mismatch: " << cblock.mask_size << " ? " << *maskSize << ")."); return 0; } diff --git a/src/tests/stub_CacheDigest.cc b/src/tests/stub_CacheDigest.cc index bbdce5fbdbd..4c865c21497 100644 --- a/src/tests/stub_CacheDigest.cc +++ b/src/tests/stub_CacheDigest.cc @@ -29,5 +29,5 @@ double CacheDigest::usedMaskPercent() const STUB_RETVAL(0.0) void cacheDigestGuessStatsUpdate(CacheDigestGuessStats *, int, int) STUB void cacheDigestGuessStatsReport(const CacheDigestGuessStats *, StoreEntry *, const SBuf &) STUB void cacheDigestReport(CacheDigest *, const SBuf &, StoreEntry *) STUB -uint32_t CacheDigest::CalcMaskSize(uint64_t, uint8_t) STUB_RETVAL(1) +std::optional CacheDigest::CalcMaskSize(uint64_t, uint8_t) STUB_RETVAL(std::nullopt) From bd5097066b69eb6b5c3fa4eb7ebf66b14ba1c6fd Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Tue, 8 Sep 2026 19:07:37 -0400 Subject: [PATCH 03/15] Revised the fix to address all known in-scope problems ... with digest mask size calculations and with sender/recipient mask size calculations getting out of sync, causing legacy senders to reject digests generated by this modern/patched code. The only (pre-existing) problem this code does not solve is support for senders and receivers that have different `int` sizes. Fixed code does not assert in such environments. It rejects the digests with a level-0 cache.log message. That is good enough for now. The key in this solution is to limit calculated digest capacity (and derive mask size from that) rather than just focusing on safe mask size calculations derived from raw capacity estimates. This solution works because the recipient uses sender's digest capacity to re-calculate the mask size. If we provide the recipient with safe capacity values and a matching mask size, the legacy recipient should be happy. TODO: * Remove a temporary assertion that duplicates unsafe code. * Polish touched error messages. * Consider reducing diff (and hiding unchanged callers) by avoiding MaskSize() renaming. --- src/CacheDigest.cc | 65 +++++++++++++++++++++++++++-------- src/CacheDigest.h | 5 +-- src/peer_digest.cc | 25 ++++---------- src/store_digest.cc | 36 +++++++++++++++---- src/tests/stub_CacheDigest.cc | 2 +- 5 files changed, 90 insertions(+), 43 deletions(-) diff --git a/src/CacheDigest.cc b/src/CacheDigest.cc index 518244c3399..a84c67f990c 100644 --- a/src/CacheDigest.cc +++ b/src/CacheDigest.cc @@ -14,8 +14,6 @@ #include "Store.h" #include "store_key_md5.h" -#include - #if USE_CACHE_DIGESTS #include "CacheDigest.h" @@ -39,14 +37,13 @@ static uint32_t hashed_keys[4]; void CacheDigest::init(uint64_t newCapacity) { - assert(newCapacity > 0); + Assure(newCapacity > 0); capacity = newCapacity; - assert(bits_per_entry > 0); - const auto newMaskSz = CacheDigest::CalcMaskSize(newCapacity, bits_per_entry); - assert(newMaskSz); // assume that newCapacity and bits_per_entry have been validated w.r.t. mask size overflows (XXX?) - mask_size = *newMaskSz; - assert(mask_size > 0); + Assure(bits_per_entry > 0); + const auto newMaskSz = CacheDigest::MaskSize(newCapacity, bits_per_entry); + Assure(newMaskSz > 0); + mask_size = newMaskSz; mask = static_cast(xcalloc(mask_size,1)); debugs(70, 2, "capacity: " << capacity << " entries, bpe: " << bits_per_entry << "; size: " @@ -273,13 +270,53 @@ cacheDigestReport(CacheDigest * cd, const SBuf &label, StoreEntry * e) ); } -std::optional -CacheDigest::CalcMaskSize(uint64_t cap, uint8_t bpe) +static uint64_t +UnsafeMaskSize(const uint64_t cap, const uint8_t bpe) +{ + Assure(bpe); + + // This limit is paranoid because no instance can store enough objects to + // exceed this maximum. + const auto maxMaskSize = std::numeric_limits::max() / 8; + + // Same as ((cap*bpe + 7)/8 > maxMaskSize) but without overflowing multiplication or sum + if (cap > (maxMaskSize*8 - 7)/bpe) + return maxMaskSize; + + return (cap*bpe + 7)/8; +} + +uint32_t +CacheDigest::MaskSize(const uint64_t cap, const uint8_t bpe) { - const uint64_t bitCount = (cap * bpe) + 7; // XXX: Overflows! - if (bitCount >= std::numeric_limits::max()) - return std::nullopt; // overflow - return static_cast(bitCount / 8); + // Not zero (for now) to avoid CacheDigest::init() assertions. + const auto minMaskSize = uint32_t(1); + + // Our mask_size data member is uint32_t. That type is hard-coded in several + // places. TODO: Use a unique type name while revising related types. We + // cannot simply cap calculations at the maximum uint32_t value because we + // must also satisfy the following requirements to protect mask_size users: + // + // R1. Avoid overflows in code that does `mask_size * 8` (e.g., to compute bit positions). + // R2. Avoid overflows in legacy callers that store `mask_size * 8` as `int`. + // R3. Avoid unreasonably large memory allocations for mask storage. + // Bug 4534 fix defined 256MB allocations as "reasonable". + // R4. Ensure that the digest capacity derived from this mask size can be + // sent to legacy installations that assert that the corresponding mask + // bit count is less than INT_MAX. + // + // Some of the current limits below are mathematically redundant (e.g., R4 + // satisfies R2), but are explicitly listed to assist with safe refactoring. + // + // For a typical 32-bit `int`, this maxMaskSize is 268'435'455 bytes. + const auto maxMaskSize = std::min({ + static_cast(std::numeric_limits::max()) / 8, // R1 + static_cast(std::numeric_limits::max()) / 8, // R2 + static_cast(256)*1024*1024, // R3 + static_cast(INT_MAX - 8) / 8}); // R4 + + const auto rawMaskSize = ::UnsafeMaskSize(cap, bpe); + return std::max(minMaskSize, static_cast(std::min(rawMaskSize, maxMaskSize))); } static void diff --git a/src/CacheDigest.h b/src/CacheDigest.h index 5e976d41e38..8d65404e8c1 100644 --- a/src/CacheDigest.h +++ b/src/CacheDigest.h @@ -14,8 +14,6 @@ #include "mem/forward.h" #include "store_key_md5.h" -#include - class CacheDigestGuessStats; class StoreEntry; @@ -47,8 +45,7 @@ class CacheDigest /// calculate the size of mask required to digest up to /// a specified capacity and bitsize. - /// \returns nil on overflows (i.e. when our mask_size would not be able to safely store the computed mask size) - static std::optional CalcMaskSize(uint64_t cap, uint8_t bpe); + static uint32_t MaskSize(uint64_t cap, uint8_t bpe); private: void init(uint64_t newCapacity); diff --git a/src/peer_digest.cc b/src/peer_digest.cc index 57a46c6d7fd..4fb44d45b41 100644 --- a/src/peer_digest.cc +++ b/src/peer_digest.cc @@ -29,8 +29,6 @@ #include "tools.h" #include "util.h" -#include - /* local types */ /* local prototypes */ @@ -781,21 +779,12 @@ peerDigestSetCBlock(PeerDigest * pd, const char *buf) return 0; } - const auto maskSize = CacheDigest::CalcMaskSize(cblock.capacity, cblock.bits_per_entry); - if (!maskSize) { - // if we cannot compute maskSize, then received cblock.mask_size cannot hold that value either - using ComputedMaskSizeType = std::remove_cv_t >; - static_assert(std::numeric_limits::max() >= std::numeric_limits::max()); - debugs(72, DBG_CRITICAL, host << " digest cblock is corrupted " << - "(mask size too small: " << cblock.mask_size << " bytes for " << - cblock.capacity << " entries with " << cblock.bits_per_entry << " bpe)."); - return 0; - } - - if ((size_t)cblock.mask_size != *maskSize) { - debugs(72, DBG_CRITICAL, host << " digest cblock is corrupted " << - "(mask size mismatch: " << cblock.mask_size << " ? " << *maskSize - << ")."); + /* check consistency further */ + const auto calculatedMaskSize = CacheDigest::MaskSize(cblock.capacity, cblock.bits_per_entry); + if (size_t(cblock.mask_size) != calculatedMaskSize) { + debugs(72, DBG_CRITICAL, host << " digest cblock is corrupted or unsupported " << + "(unexpected mask size: " << cblock.mask_size << " for " << cblock.capacity << '*' << cblock.bits_per_entry << + "; expected: " << calculatedMaskSize << ")"); return 0; } @@ -810,7 +799,7 @@ peerDigestSetCBlock(PeerDigest * pd, const char *buf) * no cblock bugs below this point */ /* check size changes */ - if (pd->cd && cblock.mask_size != (ssize_t)pd->cd->mask_size) { + if (pd->cd && size_t(cblock.mask_size) != pd->cd->mask_size) { debugs(72, 2, host << " digest changed size: " << cblock.mask_size << " -> " << pd->cd->mask_size); freed_size = pd->cd->mask_size; diff --git a/src/store_digest.cc b/src/store_digest.cc index e13944f83eb..3d543ecf220 100644 --- a/src/store_digest.cc +++ b/src/store_digest.cc @@ -80,6 +80,23 @@ static EVH storeDigestSwapOutStep; static void storeDigestCBlockSwapOut(StoreEntry * e); static void storeDigestAdd(const StoreEntry *); +// XXX: Remove! +static uint64_t +UnsafeMaskSize(const uint64_t cap, const uint8_t bpe) +{ + Assure(bpe); + + // This limit is paranoid because no instance can store enough objects to + // exceed this maximum. + const auto maxMaskSize = std::numeric_limits::max() / 8; + + // Same as ((cap*bpe + 7)/8 > maxMaskSize) but without overflowing multiplication or sum + if (cap > (maxMaskSize*8 - 7)/bpe) + return maxMaskSize; + + return (cap*bpe + 7)/8; +} + /// calculates digest capacity static uint64_t storeDigestCalcCap() @@ -104,10 +121,17 @@ storeDigestCalcCap() * cap = hi_cap; */ - // Bug 4534: we still have to set an upper-limit at some reasonable value though. - // this matches cacheDigestCalcMaskSize doing (cap*bpe)+7 < INT_MAX - const uint64_t absolute_max = (INT_MAX -8) / Config.digest.bits_per_entry; - if (cap > absolute_max) { + + const auto bpe = Config.digest.bits_per_entry; + Assure(bpe); + + // Digest recipients recalculate mask size using received capacity and bpe + // values. Limit sent capacity value to keep legacy digest recipients safe. + const auto safeMaskSizeMax = CacheDigest::MaskSize(UINT64_MAX, bpe); // absolute maximum + const auto safeCapMax = uint64_t(safeMaskSizeMax) * 8 / bpe; + const auto safeCap = std::min(cap, safeCapMax); + if (cap > safeCap) { + const auto absolute_max = safeCap; // diff reducer static time_t last_loud = 0; if (last_loud < squid_curtime - 86400) { debugs(71, DBG_IMPORTANT, "WARNING: Cache Digest cannot store " << cap << " entries. Limiting to " << absolute_max); @@ -115,10 +139,10 @@ storeDigestCalcCap() } else { debugs(71, 3, "WARNING: Cache Digest cannot store " << cap << " entries. Limiting to " << absolute_max); } - cap = absolute_max; } - return cap; + Assure(UnsafeMaskSize(safeCap, bpe) == CacheDigest::MaskSize(safeCap, bpe)); + return safeCap; } #endif /* USE_CACHE_DIGESTS */ diff --git a/src/tests/stub_CacheDigest.cc b/src/tests/stub_CacheDigest.cc index 4c865c21497..1e8dcd3d38b 100644 --- a/src/tests/stub_CacheDigest.cc +++ b/src/tests/stub_CacheDigest.cc @@ -29,5 +29,5 @@ double CacheDigest::usedMaskPercent() const STUB_RETVAL(0.0) void cacheDigestGuessStatsUpdate(CacheDigestGuessStats *, int, int) STUB void cacheDigestGuessStatsReport(const CacheDigestGuessStats *, StoreEntry *, const SBuf &) STUB void cacheDigestReport(CacheDigest *, const SBuf &, StoreEntry *) STUB -std::optional CacheDigest::CalcMaskSize(uint64_t, uint8_t) STUB_RETVAL(std::nullopt) +uint32_t CacheDigest::MaskSize(uint64_t, uint8_t) STUB_RETVAL(0) From bc4f44c411206d0ef0ac512d247557ce753839d2 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Tue, 8 Sep 2026 19:11:49 -0400 Subject: [PATCH 04/15] fixup: Removed temporary (but correct) assertion It would be nice to keep that assertion, but it requires adding a public CacheDigest::UnsafeMaskSize() method, which is probably too much. --- src/store_digest.cc | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/store_digest.cc b/src/store_digest.cc index 3d543ecf220..cdd258b39fb 100644 --- a/src/store_digest.cc +++ b/src/store_digest.cc @@ -80,23 +80,6 @@ static EVH storeDigestSwapOutStep; static void storeDigestCBlockSwapOut(StoreEntry * e); static void storeDigestAdd(const StoreEntry *); -// XXX: Remove! -static uint64_t -UnsafeMaskSize(const uint64_t cap, const uint8_t bpe) -{ - Assure(bpe); - - // This limit is paranoid because no instance can store enough objects to - // exceed this maximum. - const auto maxMaskSize = std::numeric_limits::max() / 8; - - // Same as ((cap*bpe + 7)/8 > maxMaskSize) but without overflowing multiplication or sum - if (cap > (maxMaskSize*8 - 7)/bpe) - return maxMaskSize; - - return (cap*bpe + 7)/8; -} - /// calculates digest capacity static uint64_t storeDigestCalcCap() @@ -141,7 +124,6 @@ storeDigestCalcCap() } } - Assure(UnsafeMaskSize(safeCap, bpe) == CacheDigest::MaskSize(safeCap, bpe)); return safeCap; } #endif /* USE_CACHE_DIGESTS */ From 13fb3aed0e0dbb0dc7d862b7e72d346d84b431ca Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Tue, 8 Sep 2026 19:17:17 -0400 Subject: [PATCH 05/15] fixup: Fixed category of an otherwise modified debugs() messsage The script mentioned in 2022 commit d816f28d could not handle this case but applied a similar change to a nearby similar debugs(). --- src/peer_digest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/peer_digest.cc b/src/peer_digest.cc index 4fb44d45b41..89e3493ae65 100644 --- a/src/peer_digest.cc +++ b/src/peer_digest.cc @@ -782,7 +782,7 @@ peerDigestSetCBlock(PeerDigest * pd, const char *buf) /* check consistency further */ const auto calculatedMaskSize = CacheDigest::MaskSize(cblock.capacity, cblock.bits_per_entry); if (size_t(cblock.mask_size) != calculatedMaskSize) { - debugs(72, DBG_CRITICAL, host << " digest cblock is corrupted or unsupported " << + debugs(72, DBG_CRITICAL, "ERROR: " << host << " digest cblock is corrupted or unsupported " << "(unexpected mask size: " << cblock.mask_size << " for " << cblock.capacity << '*' << cblock.bits_per_entry << "; expected: " << calculatedMaskSize << ")"); return 0; From 0528c72b517fd02453f3cd5d11a0a2e4748f35bf Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Tue, 8 Sep 2026 19:24:46 -0400 Subject: [PATCH 06/15] fixup: Described new private/helper function --- src/CacheDigest.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/CacheDigest.cc b/src/CacheDigest.cc index a84c67f990c..ab5c2da9c0a 100644 --- a/src/CacheDigest.cc +++ b/src/CacheDigest.cc @@ -270,6 +270,9 @@ cacheDigestReport(CacheDigest * cd, const SBuf &label, StoreEntry * e) ); } +/// CacheDigest::MaskSize() helper to compute digest mask size without +/// accounting for any limits or restrictions other than those imposed by +/// uint64_t type/math itself. static uint64_t UnsafeMaskSize(const uint64_t cap, const uint8_t bpe) { From 571630b42f1c428c5d901d3cd29c4603ead6c791 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Tue, 8 Sep 2026 19:53:48 -0400 Subject: [PATCH 07/15] Added one more (overlapping) restriction This addition has no effect on typical 32-bit and 64-bit POSIX systems: SSIZE_MAX is far larger than other limits. --- src/CacheDigest.cc | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/CacheDigest.cc b/src/CacheDigest.cc index ab5c2da9c0a..a8fc655db28 100644 --- a/src/CacheDigest.cc +++ b/src/CacheDigest.cc @@ -302,21 +302,23 @@ CacheDigest::MaskSize(const uint64_t cap, const uint8_t bpe) // // R1. Avoid overflows in code that does `mask_size * 8` (e.g., to compute bit positions). // R2. Avoid overflows in legacy callers that store `mask_size * 8` as `int`. - // R3. Avoid unreasonably large memory allocations for mask storage. + // R3. Avoid overflows in legacy callers that cast `mask_size` to `ssize_t`. + // R4. Avoid unreasonably large memory allocations for mask storage. // Bug 4534 fix defined 256MB allocations as "reasonable". - // R4. Ensure that the digest capacity derived from this mask size can be + // R5. Ensure that the digest capacity derived from this mask size can be // sent to legacy installations that assert that the corresponding mask // bit count is less than INT_MAX. // - // Some of the current limits below are mathematically redundant (e.g., R4 + // Some of the current limits below are mathematically redundant (e.g., R5 // satisfies R2), but are explicitly listed to assist with safe refactoring. // // For a typical 32-bit `int`, this maxMaskSize is 268'435'455 bytes. const auto maxMaskSize = std::min({ static_cast(std::numeric_limits::max()) / 8, // R1 static_cast(std::numeric_limits::max()) / 8, // R2 - static_cast(256)*1024*1024, // R3 - static_cast(INT_MAX - 8) / 8}); // R4 + static_cast(std::numeric_limits::max()), // R3 + static_cast(256)*1024*1024, // R4 + static_cast(INT_MAX - 8) / 8}); // R5 const auto rawMaskSize = ::UnsafeMaskSize(cap, bpe); return std::max(minMaskSize, static_cast(std::min(rawMaskSize, maxMaskSize))); From 98daf26e6e145465cb97386c702359246e52f602 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Tue, 8 Sep 2026 19:55:55 -0400 Subject: [PATCH 08/15] Removed one paranoid limit ... because the corresponding CacheDigest::init() assertions are (and should always be) satisfied by positive capacity already. We do not have to force mask sizes to be always positive from that point of view. --- src/CacheDigest.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/CacheDigest.cc b/src/CacheDigest.cc index a8fc655db28..e1df19f3bfa 100644 --- a/src/CacheDigest.cc +++ b/src/CacheDigest.cc @@ -292,9 +292,6 @@ UnsafeMaskSize(const uint64_t cap, const uint8_t bpe) uint32_t CacheDigest::MaskSize(const uint64_t cap, const uint8_t bpe) { - // Not zero (for now) to avoid CacheDigest::init() assertions. - const auto minMaskSize = uint32_t(1); - // Our mask_size data member is uint32_t. That type is hard-coded in several // places. TODO: Use a unique type name while revising related types. We // cannot simply cap calculations at the maximum uint32_t value because we @@ -321,7 +318,7 @@ CacheDigest::MaskSize(const uint64_t cap, const uint8_t bpe) static_cast(INT_MAX - 8) / 8}); // R5 const auto rawMaskSize = ::UnsafeMaskSize(cap, bpe); - return std::max(minMaskSize, static_cast(std::min(rawMaskSize, maxMaskSize))); + return static_cast(std::min(rawMaskSize, maxMaskSize)); } static void From 94e9a16a70e1968e914e6d6d53d865ea4d82012c Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Tue, 8 Sep 2026 19:56:14 -0400 Subject: [PATCH 09/15] fixup: formatted changes sources --- src/store_digest.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/store_digest.cc b/src/store_digest.cc index cdd258b39fb..774a2a30f0c 100644 --- a/src/store_digest.cc +++ b/src/store_digest.cc @@ -104,7 +104,6 @@ storeDigestCalcCap() * cap = hi_cap; */ - const auto bpe = Config.digest.bits_per_entry; Assure(bpe); From 34d3a6239034f229ce6c8e6b409df7a4a2a38487 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Wed, 9 Sep 2026 09:14:39 -0400 Subject: [PATCH 10/15] fixup: Undo CalcMaskSize() renaming to reduce diff --- src/CacheDigest.cc | 6 +++--- src/CacheDigest.h | 2 +- src/peer_digest.cc | 2 +- src/store_digest.cc | 2 +- src/tests/stub_CacheDigest.cc | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/CacheDigest.cc b/src/CacheDigest.cc index e1df19f3bfa..c14c562e1dc 100644 --- a/src/CacheDigest.cc +++ b/src/CacheDigest.cc @@ -41,7 +41,7 @@ CacheDigest::init(uint64_t newCapacity) capacity = newCapacity; Assure(bits_per_entry > 0); - const auto newMaskSz = CacheDigest::MaskSize(newCapacity, bits_per_entry); + const auto newMaskSz = CacheDigest::CalcMaskSize(newCapacity, bits_per_entry); Assure(newMaskSz > 0); mask_size = newMaskSz; @@ -270,7 +270,7 @@ cacheDigestReport(CacheDigest * cd, const SBuf &label, StoreEntry * e) ); } -/// CacheDigest::MaskSize() helper to compute digest mask size without +/// CacheDigest::CalcMaskSize() helper to compute digest mask size without /// accounting for any limits or restrictions other than those imposed by /// uint64_t type/math itself. static uint64_t @@ -290,7 +290,7 @@ UnsafeMaskSize(const uint64_t cap, const uint8_t bpe) } uint32_t -CacheDigest::MaskSize(const uint64_t cap, const uint8_t bpe) +CacheDigest::CalcMaskSize(const uint64_t cap, const uint8_t bpe) { // Our mask_size data member is uint32_t. That type is hard-coded in several // places. TODO: Use a unique type name while revising related types. We diff --git a/src/CacheDigest.h b/src/CacheDigest.h index 8d65404e8c1..2b49ed3d435 100644 --- a/src/CacheDigest.h +++ b/src/CacheDigest.h @@ -45,7 +45,7 @@ class CacheDigest /// calculate the size of mask required to digest up to /// a specified capacity and bitsize. - static uint32_t MaskSize(uint64_t cap, uint8_t bpe); + static uint32_t CalcMaskSize(uint64_t cap, uint8_t bpe); private: void init(uint64_t newCapacity); diff --git a/src/peer_digest.cc b/src/peer_digest.cc index 89e3493ae65..bfa1f12abbf 100644 --- a/src/peer_digest.cc +++ b/src/peer_digest.cc @@ -780,7 +780,7 @@ peerDigestSetCBlock(PeerDigest * pd, const char *buf) } /* check consistency further */ - const auto calculatedMaskSize = CacheDigest::MaskSize(cblock.capacity, cblock.bits_per_entry); + const auto calculatedMaskSize = CacheDigest::CalcMaskSize(cblock.capacity, cblock.bits_per_entry); if (size_t(cblock.mask_size) != calculatedMaskSize) { debugs(72, DBG_CRITICAL, "ERROR: " << host << " digest cblock is corrupted or unsupported " << "(unexpected mask size: " << cblock.mask_size << " for " << cblock.capacity << '*' << cblock.bits_per_entry << diff --git a/src/store_digest.cc b/src/store_digest.cc index 774a2a30f0c..4a043f9bbb6 100644 --- a/src/store_digest.cc +++ b/src/store_digest.cc @@ -109,7 +109,7 @@ storeDigestCalcCap() // Digest recipients recalculate mask size using received capacity and bpe // values. Limit sent capacity value to keep legacy digest recipients safe. - const auto safeMaskSizeMax = CacheDigest::MaskSize(UINT64_MAX, bpe); // absolute maximum + const auto safeMaskSizeMax = CacheDigest::CalcMaskSize(UINT64_MAX, bpe); // absolute maximum const auto safeCapMax = uint64_t(safeMaskSizeMax) * 8 / bpe; const auto safeCap = std::min(cap, safeCapMax); if (cap > safeCap) { diff --git a/src/tests/stub_CacheDigest.cc b/src/tests/stub_CacheDigest.cc index 1e8dcd3d38b..51ec43b5e1d 100644 --- a/src/tests/stub_CacheDigest.cc +++ b/src/tests/stub_CacheDigest.cc @@ -29,5 +29,5 @@ double CacheDigest::usedMaskPercent() const STUB_RETVAL(0.0) void cacheDigestGuessStatsUpdate(CacheDigestGuessStats *, int, int) STUB void cacheDigestGuessStatsReport(const CacheDigestGuessStats *, StoreEntry *, const SBuf &) STUB void cacheDigestReport(CacheDigest *, const SBuf &, StoreEntry *) STUB -uint32_t CacheDigest::MaskSize(uint64_t, uint8_t) STUB_RETVAL(0) +uint32_t CacheDigest::CalcMaskSize(uint64_t, uint8_t) STUB_RETVAL(0) From 7227a42749802c7016d161edf895fa2145856c9c Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Wed, 9 Sep 2026 09:21:52 -0400 Subject: [PATCH 11/15] fixup: Undo improvements that were justified by now-gone renaming --- src/CacheDigest.cc | 9 +++------ src/tests/stub_CacheDigest.cc | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/CacheDigest.cc b/src/CacheDigest.cc index c14c562e1dc..a321703becc 100644 --- a/src/CacheDigest.cc +++ b/src/CacheDigest.cc @@ -37,14 +37,11 @@ static uint32_t hashed_keys[4]; void CacheDigest::init(uint64_t newCapacity) { - Assure(newCapacity > 0); - capacity = newCapacity; - - Assure(bits_per_entry > 0); const auto newMaskSz = CacheDigest::CalcMaskSize(newCapacity, bits_per_entry); - Assure(newMaskSz > 0); + assert(newCapacity > 0 && bits_per_entry > 0); + assert(newMaskSz != 0); + capacity = newCapacity; mask_size = newMaskSz; - mask = static_cast(xcalloc(mask_size,1)); debugs(70, 2, "capacity: " << capacity << " entries, bpe: " << bits_per_entry << "; size: " << mask_size << " bytes"); diff --git a/src/tests/stub_CacheDigest.cc b/src/tests/stub_CacheDigest.cc index 51ec43b5e1d..bbdce5fbdbd 100644 --- a/src/tests/stub_CacheDigest.cc +++ b/src/tests/stub_CacheDigest.cc @@ -29,5 +29,5 @@ double CacheDigest::usedMaskPercent() const STUB_RETVAL(0.0) void cacheDigestGuessStatsUpdate(CacheDigestGuessStats *, int, int) STUB void cacheDigestGuessStatsReport(const CacheDigestGuessStats *, StoreEntry *, const SBuf &) STUB void cacheDigestReport(CacheDigest *, const SBuf &, StoreEntry *) STUB -uint32_t CacheDigest::CalcMaskSize(uint64_t, uint8_t) STUB_RETVAL(0) +uint32_t CacheDigest::CalcMaskSize(uint64_t, uint8_t) STUB_RETVAL(1) From fcdb074982757a8bf6a39c8933528689dcbe3c49 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Wed, 9 Sep 2026 10:58:22 -0400 Subject: [PATCH 12/15] fixup: Found more now-out-of-scope improvements to undo --- src/CacheDigest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CacheDigest.cc b/src/CacheDigest.cc index a321703becc..183f76e3f7c 100644 --- a/src/CacheDigest.cc +++ b/src/CacheDigest.cc @@ -287,7 +287,7 @@ UnsafeMaskSize(const uint64_t cap, const uint8_t bpe) } uint32_t -CacheDigest::CalcMaskSize(const uint64_t cap, const uint8_t bpe) +CacheDigest::CalcMaskSize(uint64_t cap, uint8_t bpe) { // Our mask_size data member is uint32_t. That type is hard-coded in several // places. TODO: Use a unique type name while revising related types. We From d22675a3f8f9bab4499d81513fa742081a13b863 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Wed, 9 Sep 2026 11:06:19 -0400 Subject: [PATCH 13/15] fixup: Restrict "legacy" to old/now-fixed code Existing code still stores the number of bits as `int`. For example, CacheDigestStats::bit_on_count is an `int` and cacheDigestStats() stores bit position `pos` in an `int`. --- src/CacheDigest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CacheDigest.cc b/src/CacheDigest.cc index 183f76e3f7c..007c252f7b3 100644 --- a/src/CacheDigest.cc +++ b/src/CacheDigest.cc @@ -295,7 +295,7 @@ CacheDigest::CalcMaskSize(uint64_t cap, uint8_t bpe) // must also satisfy the following requirements to protect mask_size users: // // R1. Avoid overflows in code that does `mask_size * 8` (e.g., to compute bit positions). - // R2. Avoid overflows in legacy callers that store `mask_size * 8` as `int`. + // R2. Avoid overflows in code that stores `mask_size * 8` as `int`. // R3. Avoid overflows in legacy callers that cast `mask_size` to `ssize_t`. // R4. Avoid unreasonably large memory allocations for mask storage. // Bug 4534 fix defined 256MB allocations as "reasonable". From cc9ba2d7e10bd574dbd8daecd08fe0f40272ac64 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Wed, 9 Sep 2026 11:07:10 -0400 Subject: [PATCH 14/15] fixup: Updated stale branch-added comment --- src/CacheDigest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CacheDigest.cc b/src/CacheDigest.cc index 007c252f7b3..7a73e06d9c5 100644 --- a/src/CacheDigest.cc +++ b/src/CacheDigest.cc @@ -306,7 +306,7 @@ CacheDigest::CalcMaskSize(uint64_t cap, uint8_t bpe) // Some of the current limits below are mathematically redundant (e.g., R5 // satisfies R2), but are explicitly listed to assist with safe refactoring. // - // For a typical 32-bit `int`, this maxMaskSize is 268'435'455 bytes. + // For a typical 32-bit `int`, this maxMaskSize is 268'435'454 bytes. const auto maxMaskSize = std::min({ static_cast(std::numeric_limits::max()) / 8, // R1 static_cast(std::numeric_limits::max()) / 8, // R2 From a64e54fe4c60eb5fb7c90f5c4e1ee5b4834a08fb Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Wed, 9 Sep 2026 11:17:57 -0400 Subject: [PATCH 15/15] fixup: Avoid introducing C-specific UINT64_MAX --- src/store_digest.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/store_digest.cc b/src/store_digest.cc index 4a043f9bbb6..1a3a7dd8457 100644 --- a/src/store_digest.cc +++ b/src/store_digest.cc @@ -35,6 +35,7 @@ #include "util.h" #include +#include /* * local types @@ -109,7 +110,7 @@ storeDigestCalcCap() // Digest recipients recalculate mask size using received capacity and bpe // values. Limit sent capacity value to keep legacy digest recipients safe. - const auto safeMaskSizeMax = CacheDigest::CalcMaskSize(UINT64_MAX, bpe); // absolute maximum + const auto safeMaskSizeMax = CacheDigest::CalcMaskSize(std::numeric_limits::max(), bpe); // absolute maximum const auto safeCapMax = uint64_t(safeMaskSizeMax) * 8 / bpe; const auto safeCap = std::min(cap, safeCapMax); if (cap > safeCap) {