From e180d3c702671c7d65d32279e37295b478446097 Mon Sep 17 00:00:00 2001 From: Parker Loflin Date: Fri, 30 Oct 2026 16:13:51 -0400 Subject: [PATCH] zlib: re-enable CRC32 SIMD optimization The memory corruption that led to disabling CRC32_SIMD_SSE42_PCLMUL came from crc_fold_copy() in deps/zlib/crc_folding.c. For the final (len % 16) bytes of a copy it did a 16-byte unaligned store, writing up to 15 bytes past the destination. With gzip at level 0, deflate_stored() copies input straight into strm->next_out through read_buf() and copy_with_crc(), so the overrun landed past the end of the caller's output buffer and corrupted the heap. Chromium replaced that store with zmemcpy(), and the fix arrived here with the zlib update in 70a05103c854 (v22.0.0). With the old code, the added test aborts v19.0.0 and v20.6.0 builds every time. With the current code it passes, and the CRC32 SIMD path is clean under ASan and valgrind. The restored zlib_crc32_simd target predates clang-cl support on Windows, so also pass -msse4.2 and -mpclmul through msvs_settings, as zlib_adler32_simd does. The bundled zlib only detects CPU features in deflateInit2() and when crc32() or adler32() is called with a NULL buffer. zlib.crc32() always passes a buffer, so it used the portable code until a stream had triggered the detection. Trigger it when the binding is initialized so zlib.crc32() uses the SIMD path from the start. Refs: https://github.com/nodejs/node/issues/45268 Refs: https://github.com/nodejs/node/pull/49511 Refs: https://github.com/nodejs/node/pull/52123 Assisted-by: Claude Code Signed-off-by: Parker Loflin --- deps/zlib/zlib.gyp | 66 ++++++++++--------- src/node_zlib.cc | 6 ++ .../test-zlib-gzip-stored-output-boundary.js | 30 +++++++++ 3 files changed, 72 insertions(+), 30 deletions(-) create mode 100644 test/parallel/test-zlib-gzip-stored-output-boundary.js diff --git a/deps/zlib/zlib.gyp b/deps/zlib/zlib.gyp index 12449e7b0294..2cfd4ad3dc49 100644 --- a/deps/zlib/zlib.gyp +++ b/deps/zlib/zlib.gyp @@ -111,33 +111,41 @@ }], ], }, # zlib_arm_crc32 - # { - # 'target_name': 'zlib_crc32_simd', - # 'type': 'static_library', - # 'conditions': [ - # ['OS!="win" or clang==1', { - # 'cflags': [ - # '-msse4.2', - # '-mpclmul', - # ], - # 'xcode_settings': { - # 'OTHER_CFLAGS': [ - # '-msse4.2', - # '-mpclmul', - # ], - # }, - # }] - # ], - # 'defines': [ 'CRC32_SIMD_SSE42_PCLMUL' ], - # 'include_dirs': [ '<(ZLIB_ROOT)' ], - # 'direct_dependent_settings': { - # 'defines': [ 'CRC32_SIMD_SSE42_PCLMUL' ], - # 'include_dirs': [ '<(ZLIB_ROOT)' ], - # }, - # 'sources': [ - # ' target, Local unused, Local context, void* priv) { + // The bundled zlib only detects CPU features in deflateInit2() and when + // crc32() or adler32() is called with a NULL buffer. zlib.crc32() always + // passes a buffer, so trigger detection here to let it use the SIMD path + // before any stream has been created. + crc32(0, nullptr, 0); + Environment* env = Environment::GetCurrent(context); MakeClass::Make(env, target, "Zlib"); diff --git a/test/parallel/test-zlib-gzip-stored-output-boundary.js b/test/parallel/test-zlib-gzip-stored-output-boundary.js new file mode 100644 index 000000000000..44dda7e2b744 --- /dev/null +++ b/test/parallel/test-zlib-gzip-stored-output-boundary.js @@ -0,0 +1,30 @@ +'use strict'; + +// Regression test for https://github.com/nodejs/node/issues/45268. +// With gzip and level 0, deflate_stored() copies input directly into the +// output buffer through the SIMD CRC32 copy routine. An old version of that +// routine stored 16 bytes for the final (len % 16) bytes and wrote past the +// end of the output buffer, corrupting the heap. Exercise output buffers that +// end at every offset modulo 16 and check the round trip. + +require('../common'); +const assert = require('assert'); +const zlib = require('zlib'); + +for (let i = 0; i < 400; i++) { + const size = 40000 + i * 517; + const input = Buffer.alloc(size, i & 0xff); + for (let j = 0; j < size; j += 97) input[j] = j & 0xff; + const chunkSize = 65536 + (i % 17); + const gz = zlib.gzipSync(input, { level: 0, chunkSize }); + assert.deepStrictEqual(zlib.gunzipSync(gz), input); +} + +// Small inputs finished straight into a nearly full output buffer. +for (let len = 1; len <= 64; len++) { + const input = Buffer.alloc(len, 0x61); + for (let chunkSize = 64; chunkSize <= 96; chunkSize++) { + const gz = zlib.gzipSync(input, { level: 0, chunkSize }); + assert.deepStrictEqual(zlib.gunzipSync(gz), input); + } +}