Memory corruption and crash when streaming to zlib

[MaddieLowe]

Version

v19.0.0

Platform

Linux 5.4.0-131-generic #147-Ubuntu SMP Fri Oct 14 17:07:22 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

The bug happens when I send certain data in random-sized chunks over a socket and then pipe it to gzip. I tried to reproduce it with a smaller data sample, but unfortunately it seems that very specific data is needed to reproduce this bug. The script and data to reproduce the issue are here: https://github.com/morpheus-med/node-defect. Repro steps are:

• Clone the repo https://github.com/morpheus-med/node-defect
• Run unzip anonymized-data.zip
• Run node index.js

How often does it reproduce? Is there a required condition?

It should crash in 1-2 runs. It crashes every time for me.

What is the expected behavior?

The expected behaviour is for the program to read and pipe the data to gzip without crashing.

What do you see instead?

The program crashes with:

double free or corruption (!prev)
Aborted (core dumped)
or
node: malloc.c:4036: _int_malloc: Assertion (unsigned long) (size) >= (unsigned long) (nb)' failed.
Aborted (core dumped)
or
corrupted size vs. prev_size
Aborted (core dumped)

Additional information

The application this is currently affecting is used for streaming medical imaging data to a viewer. We've currently got around the problem by downgrading node, but we would like to be able to upgrade node in the future to keep up with security updates.

The problem isn't reproducible with node 12.16.3, but is reproducible with versions 12.17.0 and later. I've done a git bisect and it seems to have been introduced with this commit: 9e33f97

Thanks for your help

[lpinca]

I can reproduce on Node.js main but it seems to be fixed by #44412.

[lpinca]

I can no longer reproduce the issue with Node.js 19.1.0. Can you please confirm?

We also landed #45387 (not included in a release yet) and I also cannot reproduce the issue with Node.js main.

[lpinca]

I can no longer reproduce the issue with Node.js 19.1.0. Can you please confirm?

We also landed #45387 (not included in a release yet) and I also cannot reproduce the issue with Node.js main.

Scratch that. I can reproduce with both Node.js 19.1.0 and Node.js main on Linux.

[lpinca]

This patch seems to fix the issue

diff --git a/deps/zlib/zlib.gyp b/deps/zlib/zlib.gyp
index 547143e19d..50c8ae8be4 100644
--- a/deps/zlib/zlib.gyp
+++ b/deps/zlib/zlib.gyp
@@ -82,7 +82,6 @@
               'defines': [
                 'ADLER32_SIMD_SSSE3',
                 'INFLATE_CHUNK_SIMD_SSE2',
-                'CRC32_SIMD_SSE42_PCLMUL',
                 'DEFLATE_SLIDE_HASH_SSE2'
               ],
               'sources': [

but I' not sure if the issue is Node.js or in the zlib fork we use.

[ywave620]

I can not reproduce it in v20.0.0-pre and v16.14.1. Are you still suffering from this issue? @MaddieLowe

[lpinca]

@ywave620 I can still reproduce with Node.js main.

[ywave620]

Then it's weird🤔️, I ran the script with the main version of Node in Linux VM-79-215-centos 3.10.107-1-tlinux2_kvm_guest-0049 #1 SMP Tue Jul 30 23:46:29 CST 2019 x86_64 x86_64 x86_64 GNU/Linux multiple times, but I didn't find any crash.

[lpinca]

I can reproduce on every run on WSL and sometimes on macOS (13.1). I've also received this from a failed assertion in a recent run

/Users/luigi/code/node/node[1577]: ../src/node_zlib.cc:270:virtual node::(anonymous namespace)::CompressionStream<node::(anonymous namespace)::ZlibContext>::~CompressionStream() [CompressionContext = node::(anonymous namespace)::ZlibContext]: Assertion `(zlib_memory_) == (0)' failed.
 1: 0x10c6b9645 node::Abort() [/Users/luigi/code/node/out/Release/node]
 2: 0x10c6b9451 node::Assert(node::AssertionInfo const&) [/Users/luigi/code/node/out/Release/node]
 3: 0x10c77cdd4 node::(anonymous namespace)::ZlibStream::~ZlibStream() [/Users/luigi/code/node/out/Release/node]
 4: 0x10c9e50b5 unsigned long v8::internal::GlobalHandles::InvokeFirstPassWeakCallbacks<v8::internal::GlobalHandles::Node>(std::__1::vector<std::__1::pair<v8::internal::GlobalHandles::Node*, v8::internal::GlobalHandles::PendingPhantomCallback>, std::__1::allocator<std::__1::pair<v8::internal::GlobalHandles::Node*, v8::internal::GlobalHandles::PendingPhantomCallback> > >*) [/Users/luigi/code/node/out/Release/node]
 5: 0x10ca49f73 v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::internal::GarbageCollectionReason, char const*) [/Users/luigi/code/node/out/Release/node]
 6: 0x10ca47239 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/Users/luigi/code/node/out/Release/node]
 7: 0x10cae6461 v8::internal::ScavengeJob::Task::RunInternal() [/Users/luigi/code/node/out/Release/node]
 8: 0x10c723f2e node::PerIsolatePlatformData::RunForegroundTask(std::__1::unique_ptr<v8::Task, std::__1::default_delete<v8::Task> >) [/Users/luigi/code/node/out/Release/node]
 9: 0x10c722857 node::PerIsolatePlatformData::FlushForegroundTasksInternal() [/Users/luigi/code/node/out/Release/node]
10: 0x10d22c79b uv__async_io [/Users/luigi/code/node/out/Release/node]
11: 0x10d2408e8 uv__io_poll [/Users/luigi/code/node/out/Release/node]
12: 0x10d22cc78 uv_run [/Users/luigi/code/node/out/Release/node]
13: 0x10c5ed713 node::SpinEventLoopInternal(node::Environment*) [/Users/luigi/code/node/out/Release/node]
14: 0x10c6fcde6 node::NodeMainInstance::Run() [/Users/luigi/code/node/out/Release/node]
15: 0x10c680f64 node::LoadSnapshotDataAndRun(node::SnapshotData const**, node::InitializationResultImpl const*) [/Users/luigi/code/node/out/Release/node]
16: 0x10c6811cd node::Start(int, char**) [/Users/luigi/code/node/out/Release/node]
17: 0x7ff801a73310 start [/usr/lib/dyld]
Abort trap: 6

I'm not sure if it is related.

[ywave620]

After a stress test in Darwin MacBook-Pro.local 21.6.0 Darwin Kernel Version 21.6.0: Wed Aug 10 14:25:27 PDT 2022; root:xnu-8020.141.5~2/RELEASE_X86_64 x86_64, I got a different assertion error

...
sending 759 bytes, buf size is 204747767
parse_chunk, length 759, total bytes received 1943006
sending 4381 bytes, buf size is 204747008
parse_chunk, length 4381, total bytes received 1947387
sending 3557 bytes, buf size is 204742627
parse_chunk, length 3557, total bytes received 1950944
sending 4974 bytes, buf size is 204739070
parse_chunk, length 4974, total bytes received 1955918
/Users/roger/Work/node-contribute/out/Debug/node[81933]: ../src/node_zlib.cc:752:void node::(anonymous namespace)::ZlibContext::Close(): Assertion `status == 0 || status == (-3)' failed.
 1: 0x1015e411d node::DumpBacktrace(__sFILE*) [/Users/roger/Work/node-contribute/out/Debug/node]
 2: 0x10176223b node::Abort() [/Users/roger/Work/node-contribute/out/Debug/node]
 3: 0x101761d95 node::Assert(node::AssertionInfo const&) [/Users/roger/Work/node-contribute/out/Debug/node]
 4: 0x101989385 node::(anonymous namespace)::ZlibContext::Close() [/Users/roger/Work/node-contribute/out/Debug/node]
 5: 0x10198917e node::(anonymous namespace)::CompressionStream<node::(anonymous namespace)::ZlibContext>::Close() [/Users/roger/Work/node-contribute/out/Debug/node]
 6: 0x101985141 node::(anonymous namespace)::CompressionStream<node::(anonymous namespace)::ZlibContext>::Close(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/roger/Work/node-contribute/out/Debug/node]
 7: 0x101d04e12 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [/Users/roger/Work/node-contribute/out/Debug/node]
 8: 0x101d03b3f v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, unsigned long*, int) [/Users/roger/Work/node-contribute/out/Debug/node]
 9: 0x101d027db v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [/Users/roger/Work/node-contribute/out/Debug/node]
10: 0x102d562f9 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit [/Users/roger/Work/node-contribute/out/Debug/node]

[ywave620]

Given that the original crash message provided by MaddieLowe, I suspect that bad memory allocation/deallocation is to blame. Since node performs (de)compression in bg thread, I wonder are malloc and free thread safe? Folks in https://stackoverflow.com/questions/855763/is-malloc-thread-safe confirm that the one in glibc is thread safe, how about the one in mac/windows?

[ywave620]

Given that the original crash message provided by MaddieLowe, I suspect that bad memory allocation/deallocation is to blame. Since node performs (de)compression in bg thread, I wonder are malloc and free thread safe? Folks in https://stackoverflow.com/questions/855763/is-malloc-thread-safe confirm that the one in glibc is thread safe, how about the one in mac/windows?

Yes, they are thread safe in mac(See https://opensource.apple.com/source/libmalloc/libmalloc-53.1.1/src/malloc.c.auto.html)

[bnoordhuis]

@lpinca reported removing CRC32_SIMD_SSE42_PCLMUL fixed it for him and that sounds plausible.

Just a hunch, but the xmm0-xmm15 registers are caller-saved. The SIMD version of zlib's crc32 definitely clobbers them. Maybe something somewhere is not saving the xmm registers when it should?

An easy way to test is to run under qemu twice, once with CPU = Intel Nehalem, then CPU = Intel Westmere.

If the first one works okay and the second one crashes, then that's a pretty good indicator; Westmere was when PCLMULQDQ was introduced.

[ywave620]

@MaddieLowe Could you share some thought on why you write this project in the first place? what problem you want to conquer? how you come up with the anonymized-data, because I can not identify any straightforward purpose by reading the code. It may be helpful for debugging :)

[lpinca]

@ywave620 from the issue description

The application this is currently affecting is used for streaming medical imaging data to a viewer.