From 19ec212cb3ed9485b7e25ed792c766f96e6857d2 Mon Sep 17 00:00:00 2001 From: Mulugeta Mammo Date: Tue, 28 Jul 2026 20:40:20 +0000 Subject: [PATCH 1/4] Fix trivial undefined behavior - Guard against division by zero in PrintStats() when log_stats_samples config is set to 0. - Change 'static std::mutex log_mutex' to 'inline std::mutex log_mutex' in logging.h so all TUs share one mutex instance, preventing a data race on the shared log stream. - Replace type-punned uint32_t pointer casts with memcpy in utils.cpp (DetectGzipExt) and iaa.cpp (CompressIAA) to avoid misaligned access and strict-aliasing violations. - In CompressIAA, perform the available_out bounds check before advancing next_out_ptr, avoiding formation of an out-of-bounds pointer. Signed-off-by: Mulugeta Mammo --- iaa.cpp | 10 ++++++---- logging.h | 2 +- statistics.cpp | 3 +++ utils.cpp | 5 +++-- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/iaa.cpp b/iaa.cpp index 2bbcd27..34d4475 100644 --- a/iaa.cpp +++ b/iaa.cpp @@ -6,6 +6,7 @@ #include "iaa.h" #include +#include #include #include #include @@ -90,8 +91,8 @@ int CompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output, uint32_t output_shift = 0; if (gzip_ext) { - job->next_out_ptr += GZIP_EXT_XHDR_SIZE; if (job->available_out >= GZIP_EXT_XHDR_SIZE) { + job->next_out_ptr += GZIP_EXT_XHDR_SIZE; job->available_out -= GZIP_EXT_XHDR_SIZE; } else { return 1; @@ -158,10 +159,11 @@ int CompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output, output[pos++] = 8; // LEN output[pos++] = 0; - *(uint32_t*)(output + pos) = *input_length; + uint32_t tmp_val = *input_length; + std::memcpy(output + pos, &tmp_val, sizeof(uint32_t)); pos += 4; - *(uint32_t*)(output + pos) = - *output_length - header_length - GetTrailerLength(format); + tmp_val = *output_length - header_length - GetTrailerLength(format); + std::memcpy(output + pos, &tmp_val, sizeof(uint32_t)); pos += 4; *output_length += GZIP_EXT_XHDR_SIZE; diff --git a/logging.h b/logging.h index 6a151ff..2ab2d25 100644 --- a/logging.h +++ b/logging.h @@ -51,7 +51,7 @@ inline std::ostream& GetLogStream() { #ifdef DEBUG_LOG -static std::mutex log_mutex; +inline std::mutex log_mutex; template inline void Log(LogLevel level, Args&&... args) { std::lock_guard lock(log_mutex); diff --git a/statistics.cpp b/statistics.cpp index 9dabeb6..633208b 100644 --- a/statistics.cpp +++ b/statistics.cpp @@ -25,6 +25,9 @@ const std::array stat_names{ thread_local std::array stats{}; void PrintStats() { + if (configs[LOG_STATS_SAMPLES] == 0) { + return; + } auto total_operations = stats[static_cast(Statistic::DEFLATE_COUNT)] + stats[static_cast(Statistic::INFLATE_COUNT)]; if (total_operations % configs[LOG_STATS_SAMPLES] != 0) { diff --git a/utils.cpp b/utils.cpp index 82ff618..969e12d 100644 --- a/utils.cpp +++ b/utils.cpp @@ -4,6 +4,7 @@ #include "utils.h" #include +#include CompressedFormat GetCompressedFormat(int window_bits) { if (window_bits >= -15 && window_bits <= -8) { @@ -86,7 +87,7 @@ bool DetectGzipExt(uint8_t* data, uint32_t len, uint32_t* src_size, } // Extract sizes from extended header - *src_size = *(reinterpret_cast(data + 16)); - *dest_size = *(reinterpret_cast(data + 20)); + std::memcpy(src_size, data + 16, sizeof(uint32_t)); + std::memcpy(dest_size, data + 20, sizeof(uint32_t)); return true; } From f82fb9ef703d37bf8da722b2a3aefdc5844cd7ac Mon Sep 17 00:00:00 2001 From: Olasoji Date: Wed, 29 Jul 2026 15:28:28 -0700 Subject: [PATCH 2/4] config: document log_stats_samples=0 disabling statistics The division-by-zero guard in PrintStats() gives log_stats_samples=0 a meaning it did not have before: statistics are no longer appended to the log. Document that, along with what a sample is, since a value of 0 is otherwise indistinguishable from a typo in the documented 0-INT_MAX range. Setting log_stats_samples=0 is the only way to stop statistics output without rebuilding: there is no use_statistics option, PrintStats() is called unconditionally from deflate() and inflate() when built with ENABLE_STATISTICS=ON, and LogStats() does not consult log_level. Also add log_stats_samples to config/default_config, which listed every other option in config_names[] but this one. Signed-off-by: Olasoji --- README.md | 4 +++- config/default_config | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 6941c2d..2499a0d 100644 --- a/README.md +++ b/README.md @@ -219,7 +219,9 @@ log_level log_stats_samples - Values: 0-INT_MAX. Default 1000 -- Append statistics to log every N samples (this option specifies N). +- This option applies only if the shim is built with ENABLE_STATISTICS=ON. +- Append statistics to log every N samples (this option specifies N). A sample is one deflate() or inflate() call. +- If set to 0, statistics are not appended to the log. log_file - Values: path. Default: /tmp/zlib-accel.log diff --git a/config/default_config b/config/default_config index c735aa2..353e3de 100644 --- a/config/default_config +++ b/config/default_config @@ -13,4 +13,5 @@ qat_compression_allow_chunking = 0 ignore_zlib_dictionary = 0 map_shards = 64 log_level = 1 +log_stats_samples = 1000 log_file = /tmp/zlib-accel.log From be697189d6bf63fb6a14484cb1b9fa26bb7b9b16 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Wed, 29 Jul 2026 15:32:11 -0700 Subject: [PATCH 3/4] logging: serialize LogStats() output with the log mutex LogStats() writes to the stream returned by GetLogStream() without holding any lock, so it can interleave with concurrent Log() calls writing to that same stream. Making log_mutex shared across translation units fixed the Log()-vs-Log() race but left this one. Move log_mutex out of the DEBUG_LOG block into the combined DEBUG_LOG || ENABLE_STATISTICS block so both Log() and LogStats() can lock it. Locking it from LogStats() where it stood would not compile in a DEBUG_LOG=OFF, ENABLE_STATISTICS=ON build, which CI builds. Signed-off-by: Olasoji --- logging.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/logging.h b/logging.h index 2ab2d25..9d86266 100644 --- a/logging.h +++ b/logging.h @@ -47,11 +47,14 @@ inline std::ostream& GetLogStream() { return std::cout; } +// Serializes writes to the stream returned by GetLogStream(). Shared by Log() +// and LogStats(), which may write to the same stream from different threads. +inline std::mutex log_mutex; + #endif // DEBUG_LOG || ENABLE_STATISTICS #ifdef DEBUG_LOG -inline std::mutex log_mutex; template inline void Log(LogLevel level, Args&&... args) { std::lock_guard lock(log_mutex); @@ -92,6 +95,7 @@ inline void Log(LogLevel level, Args&&... args) { template inline void LogStats(Args&&... args) { + std::lock_guard lock(log_mutex); std::ostream& stream = GetLogStream(); stream << "Stats:\n"; (..., (stream << args)); From 88ad8689eb08350a37b1fe14c7ad5e13bea0b5a5 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Wed, 29 Jul 2026 15:32:20 -0700 Subject: [PATCH 4/4] iaa: copy input_length directly in the gzip-ext header write input_length is already a uint32_t*, so the temporary is only needed for the computed length that follows. No functional change. Signed-off-by: Olasoji --- iaa.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/iaa.cpp b/iaa.cpp index 34d4475..92e5c09 100644 --- a/iaa.cpp +++ b/iaa.cpp @@ -159,10 +159,10 @@ int CompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output, output[pos++] = 8; // LEN output[pos++] = 0; - uint32_t tmp_val = *input_length; - std::memcpy(output + pos, &tmp_val, sizeof(uint32_t)); + std::memcpy(output + pos, input_length, sizeof(uint32_t)); pos += 4; - tmp_val = *output_length - header_length - GetTrailerLength(format); + uint32_t tmp_val = + *output_length - header_length - GetTrailerLength(format); std::memcpy(output + pos, &tmp_val, sizeof(uint32_t)); pos += 4;