From 3a08a7a6139926a71cf5a3865e91300ee3aa975a Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 18 Feb 2019 22:58:27 +0100 Subject: [PATCH] src: allocate Buffer memory using ArrayBuffer allocator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Always use the right allocator for memory that is turned into an `ArrayBuffer` at a later point. This enables embedders to use their own `ArrayBuffer::Allocator`s, and is inspired by Electron’s electron/node@f61bae3440e. It should render their downstream patch unnecessary. Refs: https://github.com/electron/node/commit/f61bae3440e1bfcc83bba6ff0785adfb89b4045e PR-URL: https://github.com/nodejs/node/pull/26207 Reviewed-By: James M Snell Reviewed-By: Joyee Cheung --- src/node_buffer.cc | 108 ++++++-------- src/node_crypto.cc | 283 ++++++++++++++++-------------------- src/node_crypto.h | 15 +- src/node_http2.cc | 23 ++- src/node_http2.h | 1 + src/node_http_parser_impl.h | 5 +- src/node_internals.h | 12 +- src/node_messaging.cc | 8 + src/node_native_module.cc | 10 +- src/node_serdes.cc | 5 +- src/stream_base-inl.h | 15 +- src/stream_base.cc | 47 +++--- src/stream_base.h | 14 +- src/stream_pipe.cc | 15 +- src/stream_pipe.h | 2 +- src/udp_wrap.cc | 22 ++- 16 files changed, 258 insertions(+), 327 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index b17ba2743bbfc0..e932c4a80126e6 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -54,17 +54,6 @@ size_t length = end - start; namespace node { - -namespace { - -inline void* BufferMalloc(size_t length) { - return per_process::cli_options->zero_fill_all_buffers ? - node::UncheckedCalloc(length) : - node::UncheckedMalloc(length); -} - -} // namespace - namespace Buffer { using v8::ArrayBuffer; @@ -260,7 +249,7 @@ MaybeLocal New(Isolate* isolate, char* data = nullptr; if (length > 0) { - data = static_cast(BufferMalloc(length)); + data = UncheckedMalloc(length); if (data == nullptr) return Local(); @@ -276,13 +265,7 @@ MaybeLocal New(Isolate* isolate, } } - Local buf; - if (New(isolate, data, actual).ToLocal(&buf)) - return scope.Escape(buf); - - // Object failed to be created. Clean up resources. - free(data); - return Local(); + return scope.EscapeMaybe(New(isolate, data, actual)); } @@ -308,28 +291,15 @@ MaybeLocal New(Environment* env, size_t length) { return Local(); } - void* data; + AllocatedBuffer ret(env); if (length > 0) { - data = BufferMalloc(length); - if (data == nullptr) + ret = env->AllocateManaged(length, false); + if (ret.data() == nullptr) { return Local(); - } else { - data = nullptr; - } - - Local ab = - ArrayBuffer::New(env->isolate(), - data, - length, - ArrayBufferCreationMode::kInternalized); - MaybeLocal ui = Buffer::New(env, ab, 0, length); - - if (ui.IsEmpty()) { - // Object failed to be created. Clean up resources. - free(data); + } } - return scope.Escape(ui.FromMaybe(Local())); + return scope.EscapeMaybe(ret.ToBuffer()); } @@ -355,30 +325,17 @@ MaybeLocal Copy(Environment* env, const char* data, size_t length) { return Local(); } - void* new_data; + AllocatedBuffer ret(env); if (length > 0) { CHECK_NOT_NULL(data); - new_data = node::UncheckedMalloc(length); - if (new_data == nullptr) + ret = env->AllocateManaged(length, false); + if (ret.data() == nullptr) { return Local(); - memcpy(new_data, data, length); - } else { - new_data = nullptr; - } - - Local ab = - ArrayBuffer::New(env->isolate(), - new_data, - length, - ArrayBufferCreationMode::kInternalized); - MaybeLocal ui = Buffer::New(env, ab, 0, length); - - if (ui.IsEmpty()) { - // Object failed to be created. Clean up resources. - free(new_data); + } + memcpy(ret.data(), data, length); } - return scope.Escape(ui.FromMaybe(Local())); + return scope.EscapeMaybe(ret.ToBuffer()); } @@ -423,7 +380,8 @@ MaybeLocal New(Environment* env, return scope.Escape(ui.ToLocalChecked()); } - +// Warning: This function needs `data` to be allocated with malloc() and not +// necessarily isolate's ArrayBuffer::Allocator. MaybeLocal New(Isolate* isolate, char* data, size_t length) { EscapableHandleScope handle_scope(isolate); Environment* env = Environment::GetCurrent(isolate); @@ -433,18 +391,37 @@ MaybeLocal New(Isolate* isolate, char* data, size_t length) { return MaybeLocal(); } Local obj; - if (Buffer::New(env, data, length).ToLocal(&obj)) + if (Buffer::New(env, data, length, true).ToLocal(&obj)) return handle_scope.Escape(obj); return Local(); } - -MaybeLocal New(Environment* env, char* data, size_t length) { +// Warning: If this call comes through the public node_buffer.h API, +// the contract for this function is that `data` is allocated with malloc() +// and not necessarily isolate's ArrayBuffer::Allocator. +MaybeLocal New(Environment* env, + char* data, + size_t length, + bool uses_malloc) { if (length > 0) { CHECK_NOT_NULL(data); CHECK(length <= kMaxLength); } + if (uses_malloc) { + if (env->isolate_data()->uses_node_allocator()) { + // We don't know for sure that the allocator is malloc()-based, so we need + // to fall back to the FreeCallback variant. + auto free_callback = [](char* data, void* hint) { free(data); }; + return New(env, data, length, free_callback, nullptr); + } else { + // This is malloc()-based, so we can acquire it into our own + // ArrayBufferAllocator. + CHECK_NOT_NULL(env->isolate_data()->node_allocator()); + env->isolate_data()->node_allocator()->RegisterPointer(data, length); + } + } + Local ab = ArrayBuffer::New(env->isolate(), data, @@ -1051,15 +1028,13 @@ static void EncodeUtf8String(const FunctionCallbackInfo& args) { Local str = args[0].As(); size_t length = str->Utf8Length(isolate); - char* data = node::UncheckedMalloc(length); + AllocatedBuffer buf = env->AllocateManaged(length); str->WriteUtf8(isolate, - data, + buf.data(), -1, // We are certain that `data` is sufficiently large nullptr, String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8); - auto array_buf = ArrayBuffer::New( - isolate, data, length, ArrayBufferCreationMode::kInternalized); - auto array = Uint8Array::New(array_buf, 0, length); + auto array = Uint8Array::New(buf.ToArrayBuffer(), 0, length); args.GetReturnValue().Set(array); } @@ -1121,7 +1096,8 @@ void Initialize(Local target, // It can be a nullptr when running inside an isolate where we // do not own the ArrayBuffer allocator. - if (uint32_t* zero_fill_field = env->isolate_data()->zero_fill_field()) { + if (ArrayBufferAllocator* allocator = env->isolate_data()->node_allocator()) { + uint32_t* zero_fill_field = allocator->zero_fill_field(); Local array_buffer = ArrayBuffer::New( env->isolate(), zero_fill_field, sizeof(*zero_fill_field)); CHECK(target diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 84db0f4bd766d5..ffc4ad79f93a0e 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -52,15 +52,6 @@ static const int X509_NAME_FLAGS = ASN1_STRFLGS_ESC_CTRL | XN_FLAG_FN_SN; namespace node { -namespace Buffer { -// OpenSSL uses `unsigned char*` for raw data, make this easier for us. -v8::MaybeLocal New(Environment* env, unsigned char* udata, - size_t length) { - char* data = reinterpret_cast(udata); - return Buffer::New(env, data, length); -} -} // namespace Buffer - namespace crypto { using v8::Array; @@ -1624,13 +1615,18 @@ static MaybeLocal ECPointToBuffer(Environment* env, if (error != nullptr) *error = "Failed to get public key length"; return MaybeLocal(); } - MallocedBuffer buf(len); - len = EC_POINT_point2oct(group, point, form, buf.data, buf.size, nullptr); + AllocatedBuffer buf = env->AllocateManaged(len); + len = EC_POINT_point2oct(group, + point, + form, + reinterpret_cast(buf.data()), + buf.size(), + nullptr); if (len == 0) { if (error != nullptr) *error = "Failed to get public key"; return MaybeLocal(); } - return Buffer::New(env, buf.release(), len); + return buf.ToBuffer(); } @@ -2009,9 +2005,9 @@ void SSLWrap::GetFinished(const FunctionCallbackInfo& args) { if (len == 0) return; - char* buf = Malloc(len); - CHECK_EQ(len, SSL_get_finished(w->ssl_.get(), buf, len)); - args.GetReturnValue().Set(Buffer::New(env, buf, len).ToLocalChecked()); + AllocatedBuffer buf = env->AllocateManaged(len); + CHECK_EQ(len, SSL_get_finished(w->ssl_.get(), buf.data(), len)); + args.GetReturnValue().Set(buf.ToBuffer().ToLocalChecked()); } @@ -2032,9 +2028,9 @@ void SSLWrap::GetPeerFinished(const FunctionCallbackInfo& args) { if (len == 0) return; - char* buf = Malloc(len); - CHECK_EQ(len, SSL_get_peer_finished(w->ssl_.get(), buf, len)); - args.GetReturnValue().Set(Buffer::New(env, buf, len).ToLocalChecked()); + AllocatedBuffer buf = env->AllocateManaged(len); + CHECK_EQ(len, SSL_get_peer_finished(w->ssl_.get(), buf.data(), len)); + args.GetReturnValue().Set(buf.ToBuffer().ToLocalChecked()); } @@ -2052,10 +2048,10 @@ void SSLWrap::GetSession(const FunctionCallbackInfo& args) { int slen = i2d_SSL_SESSION(sess, nullptr); CHECK_GT(slen, 0); - char* sbuf = Malloc(slen); - unsigned char* p = reinterpret_cast(sbuf); + AllocatedBuffer sbuf = env->AllocateManaged(slen); + unsigned char* p = reinterpret_cast(sbuf.data()); i2d_SSL_SESSION(sess, &p); - args.GetReturnValue().Set(Buffer::New(env, sbuf, slen).ToLocalChecked()); + args.GetReturnValue().Set(sbuf.ToBuffer().ToLocalChecked()); } @@ -3936,11 +3932,9 @@ void CipherBase::SetAAD(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(b); // Possibly report invalid state failure } - CipherBase::UpdateResult CipherBase::Update(const char* data, int len, - unsigned char** out, - int* out_len) { + AllocatedBuffer* out) { if (!ctx_) return kErrorState; MarkPopErrorOnReturn mark_pop_error_on_return; @@ -3958,27 +3952,27 @@ CipherBase::UpdateResult CipherBase::Update(const char* data, CHECK(MaybePassAuthTagToOpenSSL()); } - *out_len = 0; - int buff_len = len + EVP_CIPHER_CTX_block_size(ctx_.get()); + int buf_len = len + EVP_CIPHER_CTX_block_size(ctx_.get()); // For key wrapping algorithms, get output size by calling // EVP_CipherUpdate() with null output. if (kind_ == kCipher && mode == EVP_CIPH_WRAP_MODE && EVP_CipherUpdate(ctx_.get(), nullptr, - &buff_len, + &buf_len, reinterpret_cast(data), len) != 1) { return kErrorState; } - *out = Malloc(buff_len); + *out = env()->AllocateManaged(buf_len); int r = EVP_CipherUpdate(ctx_.get(), - *out, - out_len, + reinterpret_cast(out->data()), + &buf_len, reinterpret_cast(data), len); - CHECK_LE(*out_len, buff_len); + CHECK_LE(static_cast(buf_len), out->size()); + out->Resize(buf_len); // When in CCM mode, EVP_CipherUpdate will fail if the authentication tag is // invalid. In that case, remember the error and throw in final(). @@ -3996,9 +3990,8 @@ void CipherBase::Update(const FunctionCallbackInfo& args) { CipherBase* cipher; ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); - unsigned char* out = nullptr; + AllocatedBuffer out; UpdateResult r; - int out_len = 0; // Only copy the data if we have to, because it's a string if (args[0]->IsString()) { @@ -4006,15 +3999,14 @@ void CipherBase::Update(const FunctionCallbackInfo& args) { if (!decoder.Decode(env, args[0].As(), args[1], UTF8) .FromMaybe(false)) return; - r = cipher->Update(decoder.out(), decoder.size(), &out, &out_len); + r = cipher->Update(decoder.out(), decoder.size(), &out); } else { char* buf = Buffer::Data(args[0]); size_t buflen = Buffer::Length(args[0]); - r = cipher->Update(buf, buflen, &out, &out_len); + r = cipher->Update(buf, buflen, &out); } if (r != kSuccess) { - free(out); if (r == kErrorState) { ThrowCryptoError(env, ERR_get_error(), "Trying to add data in unsupported state"); @@ -4022,11 +4014,9 @@ void CipherBase::Update(const FunctionCallbackInfo& args) { return; } - CHECK(out != nullptr || out_len == 0); - Local buf = - Buffer::New(env, reinterpret_cast(out), out_len).ToLocalChecked(); + CHECK(out.data() != nullptr || out.size() == 0); - args.GetReturnValue().Set(buf); + args.GetReturnValue().Set(out.ToBuffer().ToLocalChecked()); } @@ -4046,14 +4036,13 @@ void CipherBase::SetAutoPadding(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(b); // Possibly report invalid state failure } - -bool CipherBase::Final(unsigned char** out, int* out_len) { +bool CipherBase::Final(AllocatedBuffer* out) { if (!ctx_) return false; const int mode = EVP_CIPHER_CTX_mode(ctx_.get()); - *out = Malloc( + *out = env()->AllocateManaged( static_cast(EVP_CIPHER_CTX_block_size(ctx_.get()))); if (kind_ == kDecipher && IsSupportedAuthenticatedMode(ctx_.get())) { @@ -4065,8 +4054,17 @@ bool CipherBase::Final(unsigned char** out, int* out_len) { bool ok; if (kind_ == kDecipher && mode == EVP_CIPH_CCM_MODE) { ok = !pending_auth_failed_; + *out = AllocatedBuffer(env()); // Empty buffer. } else { - ok = EVP_CipherFinal_ex(ctx_.get(), *out, out_len) == 1; + int out_len = out->size(); + ok = EVP_CipherFinal_ex(ctx_.get(), + reinterpret_cast(out->data()), + &out_len) == 1; + + if (out_len >= 0) + out->Resize(out_len); + else + *out = AllocatedBuffer(); // *out will not be used. if (ok && kind_ == kCipher && IsAuthenticatedMode()) { // In GCM mode, the authentication tag length can be specified in advance, @@ -4095,33 +4093,21 @@ void CipherBase::Final(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); if (cipher->ctx_ == nullptr) return env->ThrowError("Unsupported state"); - unsigned char* out_value = nullptr; - int out_len = -1; + AllocatedBuffer out; // Check IsAuthenticatedMode() first, Final() destroys the EVP_CIPHER_CTX. const bool is_auth_mode = cipher->IsAuthenticatedMode(); - bool r = cipher->Final(&out_value, &out_len); - - if (out_len <= 0 || !r) { - free(out_value); - out_value = nullptr; - out_len = 0; - if (!r) { - const char* msg = is_auth_mode ? - "Unsupported state or unable to authenticate data" : - "Unsupported state"; - - return ThrowCryptoError(env, - ERR_get_error(), - msg); - } + bool r = cipher->Final(&out); + + if (!r) { + const char* msg = is_auth_mode + ? "Unsupported state or unable to authenticate data" + : "Unsupported state"; + + return ThrowCryptoError(env, ERR_get_error(), msg); } - Local buf = Buffer::New( - env, - reinterpret_cast(out_value), - out_len).ToLocalChecked(); - args.GetReturnValue().Set(buf); + args.GetReturnValue().Set(out.ToBuffer().ToLocalChecked()); } @@ -4481,20 +4467,21 @@ void Sign::SignUpdate(const FunctionCallbackInfo& args) { sign->CheckThrow(err); } -static MallocedBuffer Node_SignFinal(EVPMDPointer&& mdctx, - const ManagedEVPPKey& pkey, - int padding, - int pss_salt_len) { +static AllocatedBuffer Node_SignFinal(Environment* env, + EVPMDPointer&& mdctx, + const ManagedEVPPKey& pkey, + int padding, + int pss_salt_len) { unsigned char m[EVP_MAX_MD_SIZE]; unsigned int m_len; if (!EVP_DigestFinal_ex(mdctx.get(), m, &m_len)) - return MallocedBuffer(); + return AllocatedBuffer(); int signed_sig_len = EVP_PKEY_size(pkey.get()); CHECK_GE(signed_sig_len, 0); size_t sig_len = static_cast(signed_sig_len); - MallocedBuffer sig(sig_len); + AllocatedBuffer sig = env->AllocateManaged(sig_len); EVPKeyCtxPointer pkctx(EVP_PKEY_CTX_new(pkey.get(), nullptr)); if (pkctx && @@ -4502,12 +4489,16 @@ static MallocedBuffer Node_SignFinal(EVPMDPointer&& mdctx, ApplyRSAOptions(pkey, pkctx.get(), padding, pss_salt_len) && EVP_PKEY_CTX_set_signature_md(pkctx.get(), EVP_MD_CTX_md(mdctx.get())) > 0 && - EVP_PKEY_sign(pkctx.get(), sig.data, &sig_len, m, m_len) > 0) { - sig.Truncate(sig_len); + EVP_PKEY_sign(pkctx.get(), + reinterpret_cast(sig.data()), + &sig_len, + m, + m_len) > 0) { + sig.Resize(sig_len); return sig; } - return MallocedBuffer(); + return AllocatedBuffer(); } Sign::SignResult Sign::SignFinal( @@ -4546,16 +4537,14 @@ Sign::SignResult Sign::SignFinal( } #endif // NODE_FIPS_MODE - MallocedBuffer buffer = - Node_SignFinal(std::move(mdctx), pkey, padding, salt_len); - Error error = buffer.is_empty() ? kSignPrivateKey : kSignOk; + AllocatedBuffer buffer = + Node_SignFinal(env(), std::move(mdctx), pkey, padding, salt_len); + Error error = buffer.data() == nullptr ? kSignPrivateKey : kSignOk; return SignResult(error, std::move(buffer)); } void Sign::SignFinal(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - Sign* sign; ASSIGN_OR_RETURN_UNWRAP(&sign, args.Holder()); @@ -4580,13 +4569,7 @@ void Sign::SignFinal(const FunctionCallbackInfo& args) { if (ret.error != kSignOk) return sign->CheckThrow(ret.error); - MallocedBuffer sig = - std::move(ret.signature); - - Local rc = - Buffer::New(env, reinterpret_cast(sig.release()), sig.size) - .ToLocalChecked(); - args.GetReturnValue().Set(rc); + args.GetReturnValue().Set(ret.signature.ToBuffer().ToLocalChecked()); } void Verify::Initialize(Environment* env, Local target) { @@ -4695,16 +4678,15 @@ void Verify::VerifyFinal(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(verify_result); } - template -bool PublicKeyCipher::Cipher(const ManagedEVPPKey& pkey, +bool PublicKeyCipher::Cipher(Environment* env, + const ManagedEVPPKey& pkey, int padding, const unsigned char* data, int len, - unsigned char** out, - size_t* out_len) { + AllocatedBuffer* out) { EVPKeyCtxPointer ctx(EVP_PKEY_CTX_new(pkey.get(), nullptr)); if (!ctx) return false; @@ -4713,14 +4695,21 @@ bool PublicKeyCipher::Cipher(const ManagedEVPPKey& pkey, if (EVP_PKEY_CTX_set_rsa_padding(ctx.get(), padding) <= 0) return false; - if (EVP_PKEY_cipher(ctx.get(), nullptr, out_len, data, len) <= 0) + size_t out_len = 0; + if (EVP_PKEY_cipher(ctx.get(), nullptr, &out_len, data, len) <= 0) return false; - *out = Malloc(*out_len); + *out = env->AllocateManaged(out_len); - if (EVP_PKEY_cipher(ctx.get(), *out, out_len, data, len) <= 0) + if (EVP_PKEY_cipher(ctx.get(), + reinterpret_cast(out->data()), + &out_len, + data, + len) <= 0) { return false; + } + out->Resize(out_len); return true; } @@ -4743,33 +4732,22 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo& args) { uint32_t padding; if (!args[offset + 1]->Uint32Value(env->context()).To(&padding)) return; - unsigned char* out_value = nullptr; - size_t out_len = 0; + AllocatedBuffer out; ClearErrorOnReturn clear_error_on_return; bool r = Cipher( + env, pkey, padding, reinterpret_cast(buf), len, - &out_value, - &out_len); - - if (out_len == 0 || !r) { - free(out_value); - out_value = nullptr; - out_len = 0; - if (!r) { - return ThrowCryptoError(env, - ERR_get_error()); - } - } + &out); - Local vbuf = - Buffer::New(env, reinterpret_cast(out_value), out_len) - .ToLocalChecked(); - args.GetReturnValue().Set(vbuf); + if (!r) + return ThrowCryptoError(env, ERR_get_error()); + + args.GetReturnValue().Set(out.ToBuffer().ToLocalChecked()); } @@ -4934,10 +4912,11 @@ void DiffieHellman::GenerateKeys(const FunctionCallbackInfo& args) { DH_get0_key(diffieHellman->dh_.get(), &pub_key, nullptr); const int size = BN_num_bytes(pub_key); CHECK_GE(size, 0); - char* data = Malloc(size); + AllocatedBuffer data = env->AllocateManaged(size); CHECK_EQ(size, - BN_bn2binpad(pub_key, reinterpret_cast(data), size)); - args.GetReturnValue().Set(Buffer::New(env, data, size).ToLocalChecked()); + BN_bn2binpad( + pub_key, reinterpret_cast(data.data()), size)); + args.GetReturnValue().Set(data.ToBuffer().ToLocalChecked()); } @@ -4954,10 +4933,11 @@ void DiffieHellman::GetField(const FunctionCallbackInfo& args, const int size = BN_num_bytes(num); CHECK_GE(size, 0); - char* data = Malloc(size); - CHECK_EQ(size, - BN_bn2binpad(num, reinterpret_cast(data), size)); - args.GetReturnValue().Set(Buffer::New(env, data, size).ToLocalChecked()); + AllocatedBuffer data = env->AllocateManaged(size); + CHECK_EQ( + size, + BN_bn2binpad(num, reinterpret_cast(data.data()), size)); + args.GetReturnValue().Set(data.ToBuffer().ToLocalChecked()); } void DiffieHellman::GetPrime(const FunctionCallbackInfo& args) { @@ -5015,9 +4995,9 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { Buffer::Length(args[0]), nullptr)); - MallocedBuffer data(DH_size(diffieHellman->dh_.get())); + AllocatedBuffer ret = env->AllocateManaged(DH_size(diffieHellman->dh_.get())); - int size = DH_compute_key(reinterpret_cast(data.data), + int size = DH_compute_key(reinterpret_cast(ret.data()), key.get(), diffieHellman->dh_.get()); @@ -5052,14 +5032,13 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { // DH_compute_key returns number of bytes in a remainder of exponent, which // may have less bytes than a prime number. Therefore add 0-padding to the // allocated buffer. - if (static_cast(size) != data.size) { - CHECK_GT(data.size, static_cast(size)); - memmove(data.data + data.size - size, data.data, size); - memset(data.data, 0, data.size - size); + if (static_cast(size) != ret.size()) { + CHECK_GT(ret.size(), static_cast(size)); + memmove(ret.data() + ret.size() - size, ret.data(), size); + memset(ret.data(), 0, ret.size() - size); } - args.GetReturnValue().Set( - Buffer::New(env->isolate(), data.release(), data.size).ToLocalChecked()); + args.GetReturnValue().Set(ret.ToBuffer().ToLocalChecked()); } void DiffieHellman::SetKey(const FunctionCallbackInfo& args, @@ -5233,15 +5212,14 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo& args) { // NOTE: field_size is in bits int field_size = EC_GROUP_get_degree(ecdh->group_); size_t out_len = (field_size + 7) / 8; - char* out = node::Malloc(out_len); + AllocatedBuffer out = env->AllocateManaged(out_len); - int r = ECDH_compute_key(out, out_len, pub.get(), ecdh->key_.get(), nullptr); - if (!r) { - free(out); + int r = ECDH_compute_key( + out.data(), out_len, pub.get(), ecdh->key_.get(), nullptr); + if (!r) return env->ThrowError("Failed to compute ECDH key"); - } - Local buf = Buffer::New(env, out, out_len).ToLocalChecked(); + Local buf = out.ToBuffer().ToLocalChecked(); args.GetReturnValue().Set(buf); } @@ -5283,11 +5261,12 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo& args) { return env->ThrowError("Failed to get ECDH private key"); const int size = BN_num_bytes(b); - unsigned char* out = node::Malloc(size); - CHECK_EQ(size, BN_bn2binpad(b, out, size)); + AllocatedBuffer out = env->AllocateManaged(size); + CHECK_EQ(size, BN_bn2binpad(b, + reinterpret_cast(out.data()), + size)); - Local buf = - Buffer::New(env, reinterpret_cast(out), size).ToLocalChecked(); + Local buf = out.ToBuffer().ToLocalChecked(); args.GetReturnValue().Set(buf); } @@ -6039,31 +6018,28 @@ void VerifySpkac(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(verify_result); } - -char* ExportPublicKey(const char* data, int len, size_t* size) { - char* buf = nullptr; - +AllocatedBuffer ExportPublicKey(Environment* env, + const char* data, + int len, + size_t* size) { BIOPointer bio(BIO_new(BIO_s_mem())); - if (!bio) - return nullptr; + if (!bio) return AllocatedBuffer(); NetscapeSPKIPointer spki(NETSCAPE_SPKI_b64_decode(data, len)); - if (!spki) - return nullptr; + if (!spki) return AllocatedBuffer(); EVPKeyPointer pkey(NETSCAPE_SPKI_get_pubkey(spki.get())); - if (!pkey) - return nullptr; + if (!pkey) return AllocatedBuffer(); if (PEM_write_bio_PUBKEY(bio.get(), pkey.get()) <= 0) - return nullptr; + return AllocatedBuffer(); BUF_MEM* ptr; BIO_get_mem_ptr(bio.get(), &ptr); *size = ptr->length; - buf = Malloc(*size); - memcpy(buf, ptr->data, *size); + AllocatedBuffer buf = env->AllocateManaged(*size); + memcpy(buf.data(), ptr->data, *size); return buf; } @@ -6080,12 +6056,11 @@ void ExportPublicKey(const FunctionCallbackInfo& args) { CHECK_NOT_NULL(data); size_t pkey_size; - char* pkey = ExportPublicKey(data, length, &pkey_size); - if (pkey == nullptr) + AllocatedBuffer pkey = ExportPublicKey(env, data, length, &pkey_size); + if (pkey.data() == nullptr) return args.GetReturnValue().SetEmptyString(); - Local out = Buffer::New(env, pkey, pkey_size).ToLocalChecked(); - args.GetReturnValue().Set(out); + args.GetReturnValue().Set(pkey.ToBuffer().ToLocalChecked()); } diff --git a/src/node_crypto.h b/src/node_crypto.h index e9862ff1bc5879..78293e70f1d794 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -544,9 +544,8 @@ class CipherBase : public BaseObject { bool InitAuthenticated(const char* cipher_type, int iv_len, unsigned int auth_tag_len); bool CheckCCMMessageLength(int message_len); - UpdateResult Update(const char* data, int len, unsigned char** out, - int* out_len); - bool Final(unsigned char** out, int* out_len); + UpdateResult Update(const char* data, int len, AllocatedBuffer* out); + bool Final(AllocatedBuffer* out); bool SetAutoPadding(bool auto_padding); bool IsAuthenticatedMode() const; @@ -677,11 +676,11 @@ class Sign : public SignBase { struct SignResult { Error error; - MallocedBuffer signature; + AllocatedBuffer signature; explicit SignResult( Error err, - MallocedBuffer&& sig = MallocedBuffer()) + AllocatedBuffer&& sig = AllocatedBuffer()) : error(err), signature(std::move(sig)) {} }; @@ -738,12 +737,12 @@ class PublicKeyCipher { template - static bool Cipher(const ManagedEVPPKey& pkey, + static bool Cipher(Environment* env, + const ManagedEVPPKey& pkey, int padding, const unsigned char* data, int len, - unsigned char** out, - size_t* out_len); + AllocatedBuffer* out); template AllocateManaged(suggested_size).release(); +} + // Callback used to receive inbound data from the i/o stream -void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { +void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) { HandleScope handle_scope(env()->isolate()); Context::Scope context_scope(env()->context()); Http2Scope h2scope(this); CHECK_NOT_NULL(stream_); Debug(this, "receiving %d bytes", nread); CHECK(stream_buf_ab_.IsEmpty()); + AllocatedBuffer buf(env(), buf_); // Only pass data on if nread > 0 if (nread <= 0) { - free(buf.base); if (nread < 0) { PassReadErrorToPreviousListener(nread); } @@ -1786,13 +1789,13 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { } // Shrink to the actual amount of used data. - char* base = Realloc(buf.base, nread); + buf.Resize(nread); - IncrementCurrentSessionMemory(nread); + IncrementCurrentSessionMemory(buf.size()); OnScopeLeave on_scope_leave([&]() { // Once finished handling this write, reset the stream buffer. // The memory has either been free()d or was handed over to V8. - DecrementCurrentSessionMemory(nread); + DecrementCurrentSessionMemory(buf.size()); stream_buf_ab_ = Local(); stream_buf_ = uv_buf_init(nullptr, 0); }); @@ -1803,17 +1806,13 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { // Remember the current buffer, so that OnDataChunkReceived knows the // offset of a DATA frame's data into the socket read buffer. - stream_buf_ = uv_buf_init(base, nread); + stream_buf_ = uv_buf_init(buf.data(), nread); Isolate* isolate = env()->isolate(); // Create an array buffer for the read data. DATA frames will be emitted // as slices of this array buffer to avoid having to copy memory. - stream_buf_ab_ = - ArrayBuffer::New(isolate, - base, - nread, - ArrayBufferCreationMode::kInternalized); + stream_buf_ab_ = buf.ToArrayBuffer(); statistics_.data_received += nread; ssize_t ret = Write(&stream_buf_, 1); diff --git a/src/node_http2.h b/src/node_http2.h index 660a713d19e779..aa953667facccf 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -783,6 +783,7 @@ class Http2Session : public AsyncWrap, public StreamListener { } // Handle reads/writes from the underlying network transport. + uv_buf_t OnStreamAlloc(size_t suggested_size) override; void OnStreamRead(ssize_t nread, const uv_buf_t& buf) override; void OnStreamAfterWrite(WriteWrap* w, int status) override; diff --git a/src/node_http_parser_impl.h b/src/node_http_parser_impl.h index 7d5ea347202c59..16c0e77ae853f4 100644 --- a/src/node_http_parser_impl.h +++ b/src/node_http_parser_impl.h @@ -594,10 +594,9 @@ class Parser : public AsyncWrap, public StreamListener { uv_buf_t OnStreamAlloc(size_t suggested_size) override { // For most types of streams, OnStreamRead will be immediately after // OnStreamAlloc, and will consume all data, so using a static buffer for - // reading is more efficient. For other streams, just use the default - // allocator, which uses Malloc(). + // reading is more efficient. For other streams, just use Malloc() directly. if (env()->http_parser_buffer_in_use()) - return StreamListener::OnStreamAlloc(suggested_size); + return uv_buf_init(Malloc(suggested_size), suggested_size); env()->set_http_parser_buffer_in_use(true); if (env()->http_parser_buffer() == nullptr) diff --git a/src/node_internals.h b/src/node_internals.h index 9ac2b0a331c531..82cf5713e95bf2 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -146,10 +146,12 @@ v8::MaybeLocal New(Environment* env, size_t length, void (*callback)(char* data, void* hint), void* hint); -// Takes ownership of |data|. Must allocate |data| with malloc() or realloc() -// because ArrayBufferAllocator::Free() deallocates it again with free(). -// Mixing operator new and free() is undefined behavior so don't do that. -v8::MaybeLocal New(Environment* env, char* data, size_t length); +// Takes ownership of |data|. Must allocate |data| with the current Isolate's +// ArrayBuffer::Allocator(). +v8::MaybeLocal New(Environment* env, + char* data, + size_t length, + bool uses_malloc); // Construct a Buffer from a MaybeStackBuffer (and also its subclasses like // Utf8Value and TwoByteValue). @@ -167,7 +169,7 @@ static v8::MaybeLocal New(Environment* env, const size_t len_in_bytes = buf->length() * sizeof(buf->out()[0]); if (buf->IsAllocated()) - ret = New(env, src, len_in_bytes); + ret = New(env, src, len_in_bytes, true); else if (!buf->IsInvalidated()) ret = Copy(env, src, len_in_bytes); diff --git a/src/node_messaging.cc b/src/node_messaging.cc index c659ac06f1d41d..34977557c5bfb8 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -144,6 +144,9 @@ MaybeLocal Message::Deserialize(Environment* env, continue; } + env->isolate_data()->node_allocator()->RegisterPointer( + array_buffer_contents_[i].data, array_buffer_contents_[i].size); + Local ab = ArrayBuffer::New(env->isolate(), array_buffer_contents_[i].release(), @@ -367,6 +370,11 @@ Maybe Message::Serialize(Environment* env, // it inaccessible in this Isolate. ArrayBuffer::Contents contents = ab->Externalize(); ab->Neuter(); + + CHECK(env->isolate_data()->uses_node_allocator()); + env->isolate_data()->node_allocator()->UnregisterPointer( + contents.Data(), contents.ByteLength()); + array_buffer_contents_.push_back( MallocedBuffer { static_cast(contents.Data()), contents.ByteLength() }); diff --git a/src/node_native_module.cc b/src/node_native_module.cc index 2d3769ebf5c949..df7e1749e52ea8 100644 --- a/src/node_native_module.cc +++ b/src/node_native_module.cc @@ -11,7 +11,6 @@ namespace native_module { using v8::Array; using v8::ArrayBuffer; -using v8::ArrayBufferCreationMode; using v8::Context; using v8::DEFAULT; using v8::EscapableHandleScope; @@ -153,13 +152,8 @@ MaybeLocal NativeModuleLoader::GetCodeCache(Isolate* isolate, cached_data = it->second.get(); - MallocedBuffer copied(cached_data->length); - memcpy(copied.data, cached_data->data, cached_data->length); - Local buf = - ArrayBuffer::New(isolate, - copied.release(), - cached_data->length, - ArrayBufferCreationMode::kInternalized); + Local buf = ArrayBuffer::New(isolate, cached_data->length); + memcpy(buf->GetContents().Data(), cached_data->data, cached_data->length); return scope.Escape(Uint8Array::New(buf, 0, cached_data->length)); } diff --git a/src/node_serdes.cc b/src/node_serdes.cc index 09eade4f6f6b31..8b01e8225d1fda 100644 --- a/src/node_serdes.cc +++ b/src/node_serdes.cc @@ -201,10 +201,13 @@ void SerializerContext::ReleaseBuffer(const FunctionCallbackInfo& args) { SerializerContext* ctx; ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Holder()); + // Note: Both ValueSerializer and this Buffer::New() variant use malloc() + // as the underlying allocator. std::pair ret = ctx->serializer_.Release(); auto buf = Buffer::New(ctx->env(), reinterpret_cast(ret.first), - ret.second); + ret.second, + true /* uses_malloc */); if (!buf.IsEmpty()) { args.GetReturnValue().Set(buf.ToLocalChecked()); diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index 7db8403ced832b..9cff67cd9fecf2 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -419,18 +419,9 @@ inline void ShutdownWrap::OnDone(int status) { Dispose(); } -inline void WriteWrap::SetAllocatedStorage(char* data, size_t size) { - CHECK_NULL(storage_); - storage_ = data; - storage_size_ = size; -} - -inline char* WriteWrap::Storage() { - return storage_; -} - -inline size_t WriteWrap::StorageSize() const { - return storage_size_; +inline void WriteWrap::SetAllocatedStorage(AllocatedBuffer&& storage) { + CHECK_NULL(storage_.data()); + storage_ = std::move(storage); } inline void WriteWrap::OnDone(int status) { diff --git a/src/stream_base.cc b/src/stream_base.cc index a55cb60cfc5338..ebd9beb984eba3 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -111,9 +111,9 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { } } - MallocedBuffer storage; + AllocatedBuffer storage; if (storage_size > 0) - storage = MallocedBuffer(storage_size); + storage = env->AllocateManaged(storage_size); offset = 0; if (!all_buffers) { @@ -129,8 +129,8 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { // Write string CHECK_LE(offset, storage_size); - char* str_storage = storage.data + offset; - size_t str_size = storage_size - offset; + char* str_storage = storage.data() + offset; + size_t str_size = storage.size() - offset; Local string = chunk->ToString(env->context()).ToLocalChecked(); enum encoding encoding = ParseEncoding(env->isolate(), @@ -149,7 +149,7 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { StreamWriteResult res = Write(*bufs, count, nullptr, req_wrap_obj); SetWriteResult(res); if (res.wrap != nullptr && storage_size > 0) { - res.wrap->SetAllocatedStorage(storage.release(), storage_size); + res.wrap->SetAllocatedStorage(std::move(storage)); } return res.err; } @@ -239,18 +239,18 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { CHECK_EQ(count, 1); } - MallocedBuffer data; + AllocatedBuffer data; if (try_write) { // Copy partial data - data = MallocedBuffer(buf.len); - memcpy(data.data, buf.base, buf.len); + data = env->AllocateManaged(buf.len); + memcpy(data.data(), buf.base, buf.len); data_size = buf.len; } else { // Write it - data = MallocedBuffer(storage_size); + data = env->AllocateManaged(storage_size); data_size = StringBytes::Write(env->isolate(), - data.data, + data.data(), storage_size, string, enc); @@ -258,7 +258,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { CHECK_LE(data_size, storage_size); - buf = uv_buf_init(data.data, data_size); + buf = uv_buf_init(data.data(), data_size); uv_stream_t* send_handle = nullptr; @@ -278,7 +278,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { SetWriteResult(res); if (res.wrap != nullptr) { - res.wrap->SetAllocatedStorage(data.release(), data_size); + res.wrap->SetAllocatedStorage(std::move(data)); } return res.err; @@ -343,35 +343,30 @@ void StreamResource::ClearError() { // No-op } - -uv_buf_t StreamListener::OnStreamAlloc(size_t suggested_size) { - return uv_buf_init(Malloc(suggested_size), suggested_size); +uv_buf_t EmitToJSStreamListener::OnStreamAlloc(size_t suggested_size) { + CHECK_NOT_NULL(stream_); + Environment* env = static_cast(stream_)->stream_env(); + return env->AllocateManaged(suggested_size).release(); } - -void EmitToJSStreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { +void EmitToJSStreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) { CHECK_NOT_NULL(stream_); StreamBase* stream = static_cast(stream_); Environment* env = stream->stream_env(); HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); + AllocatedBuffer buf(env, buf_); if (nread <= 0) { - free(buf.base); if (nread < 0) stream->CallJSOnreadMethod(nread, Local()); return; } - CHECK_LE(static_cast(nread), buf.len); - char* base = Realloc(buf.base, nread); + CHECK_LE(static_cast(nread), buf.size()); + buf.Resize(nread); - Local obj = ArrayBuffer::New( - env->isolate(), - base, - nread, - v8::ArrayBufferCreationMode::kInternalized); // Transfer ownership to V8. - stream->CallJSOnreadMethod(nread, obj); + stream->CallJSOnreadMethod(nread, buf.ToArrayBuffer()); } diff --git a/src/stream_base.h b/src/stream_base.h index d5b2235f0e6f71..33cf62a0c861ba 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -74,24 +74,17 @@ class ShutdownWrap : public StreamReq { class WriteWrap : public StreamReq { public: - char* Storage(); - size_t StorageSize() const; - void SetAllocatedStorage(char* data, size_t size); + void SetAllocatedStorage(AllocatedBuffer&& storage); WriteWrap(StreamBase* stream, v8::Local req_wrap_obj) : StreamReq(stream, req_wrap_obj) { } - ~WriteWrap() override { - free(storage_); - } - // Call stream()->EmitAfterWrite() and dispose of this request wrap. void OnDone(int status) override; private: - char* storage_ = nullptr; - size_t storage_size_ = 0; + AllocatedBuffer storage_; }; @@ -115,7 +108,7 @@ class StreamListener { // It is not valid to return a zero-length buffer from this method. // It is not guaranteed that the corresponding `OnStreamRead()` call // happens in the same event loop turn as this call. - virtual uv_buf_t OnStreamAlloc(size_t suggested_size); + virtual uv_buf_t OnStreamAlloc(size_t suggested_size) = 0; // `OnStreamRead()` is called when data is available on the socket and has // been read into the buffer provided by `OnStreamAlloc()`. @@ -181,6 +174,7 @@ class ReportWritesToJSStreamListener : public StreamListener { // JS land via the handle’s .ondata method. class EmitToJSStreamListener : public ReportWritesToJSStreamListener { public: + uv_buf_t OnStreamAlloc(size_t suggested_size) override; void OnStreamRead(ssize_t nread, const uv_buf_t& buf) override; }; diff --git a/src/stream_pipe.cc b/src/stream_pipe.cc index 19d732d6592aaa..4c9b447a61c826 100644 --- a/src/stream_pipe.cc +++ b/src/stream_pipe.cc @@ -114,17 +114,17 @@ uv_buf_t StreamPipe::ReadableListener::OnStreamAlloc(size_t suggested_size) { StreamPipe* pipe = ContainerOf(&StreamPipe::readable_listener_, this); size_t size = std::min(suggested_size, pipe->wanted_data_); CHECK_GT(size, 0); - return uv_buf_init(Malloc(size), size); + return pipe->env()->AllocateManaged(size).release(); } void StreamPipe::ReadableListener::OnStreamRead(ssize_t nread, - const uv_buf_t& buf) { + const uv_buf_t& buf_) { StreamPipe* pipe = ContainerOf(&StreamPipe::readable_listener_, this); + AllocatedBuffer buf(pipe->env(), buf_); AsyncScope async_scope(pipe); if (nread < 0) { // EOF or error; stop reading and pass the error to the previous listener // (which might end up in JS). - free(buf.base); pipe->is_eof_ = true; stream()->ReadStop(); CHECK_NOT_NULL(previous_listener_); @@ -138,19 +138,18 @@ void StreamPipe::ReadableListener::OnStreamRead(ssize_t nread, return; } - pipe->ProcessData(nread, buf); + pipe->ProcessData(nread, std::move(buf)); } -void StreamPipe::ProcessData(size_t nread, const uv_buf_t& buf) { - uv_buf_t buffer = uv_buf_init(buf.base, nread); +void StreamPipe::ProcessData(size_t nread, AllocatedBuffer&& buf) { + uv_buf_t buffer = uv_buf_init(buf.data(), nread); StreamWriteResult res = sink()->Write(&buffer, 1); if (!res.async) { - free(buf.base); writable_listener_.OnStreamAfterWrite(nullptr, res.err); } else { is_writing_ = true; is_reading_ = false; - res.wrap->SetAllocatedStorage(buf.base, buf.len); + res.wrap->SetAllocatedStorage(std::move(buf)); if (source() != nullptr) source()->ReadStop(); } diff --git a/src/stream_pipe.h b/src/stream_pipe.h index 749d4eabc5237b..061ad9842e8f6d 100644 --- a/src/stream_pipe.h +++ b/src/stream_pipe.h @@ -41,7 +41,7 @@ class StreamPipe : public AsyncWrap { // `OnStreamWantsWrite()` support. size_t wanted_data_ = 0; - void ProcessData(size_t nread, const uv_buf_t& buf); + void ProcessData(size_t nread, AllocatedBuffer&& buf); class ReadableListener : public StreamListener { public: diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 5702ba5ad2fe7d..449f17c9430e9f 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -461,25 +461,23 @@ void UDPWrap::OnSend(uv_udp_send_t* req, int status) { void UDPWrap::OnAlloc(uv_handle_t* handle, size_t suggested_size, uv_buf_t* buf) { - buf->base = node::Malloc(suggested_size); - buf->len = suggested_size; + UDPWrap* wrap = static_cast(handle->data); + *buf = wrap->env()->AllocateManaged(suggested_size).release(); } - void UDPWrap::OnRecv(uv_udp_t* handle, ssize_t nread, - const uv_buf_t* buf, + const uv_buf_t* buf_, const struct sockaddr* addr, unsigned int flags) { + UDPWrap* wrap = static_cast(handle->data); + Environment* env = wrap->env(); + + AllocatedBuffer buf(env, *buf_); if (nread == 0 && addr == nullptr) { - if (buf->base != nullptr) - free(buf->base); return; } - UDPWrap* wrap = static_cast(handle->data); - Environment* env = wrap->env(); - HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); @@ -492,14 +490,12 @@ void UDPWrap::OnRecv(uv_udp_t* handle, }; if (nread < 0) { - if (buf->base != nullptr) - free(buf->base); wrap->MakeCallback(env->onmessage_string(), arraysize(argv), argv); return; } - char* base = node::UncheckedRealloc(buf->base, nread); - argv[2] = Buffer::New(env, base, nread).ToLocalChecked(); + buf.Resize(nread); + argv[2] = buf.ToBuffer().ToLocalChecked(); argv[3] = AddressToJS(env, addr); wrap->MakeCallback(env->onmessage_string(), arraysize(argv), argv); }