From dea1404add303eaa84bcb616263f8dc7bf39d9e6 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 | 112 ++++++++----------- src/node_crypto.cc | 237 ++++++++++++++++++---------------------- src/node_crypto.h | 11 +- src/node_http2.cc | 30 +++-- src/node_http2.h | 1 + src/node_http_parser.cc | 5 +- src/node_internals.h | 12 +- src/node_messaging.cc | 21 ++++ src/node_serdes.cc | 5 +- src/stream_base-inl.h | 15 +-- src/stream_base.cc | 45 ++++---- src/stream_base.h | 13 +-- src/stream_pipe.cc | 15 ++- src/stream_pipe.h | 2 +- src/udp_wrap.cc | 22 ++-- 15 files changed, 255 insertions(+), 291 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index dd285156b56..2da7fcb0891 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -55,19 +55,6 @@ size_t length = end - start; namespace node { - -// if true, all Buffer and SlowBuffer instances will automatically zero-fill -bool zero_fill_all_buffers = false; - -namespace { - -inline void* BufferMalloc(size_t length) { - return zero_fill_all_buffers ? node::UncheckedCalloc(length) : - node::UncheckedMalloc(length); -} - -} // namespace - namespace Buffer { using v8::ArrayBuffer; @@ -245,7 +232,7 @@ MaybeLocal New(Isolate* isolate, char* data = nullptr; if (length > 0) { - data = static_cast(BufferMalloc(length)); + data = UncheckedMalloc(length); if (data == nullptr) return Local(); @@ -261,13 +248,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)); } @@ -290,28 +271,16 @@ 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) { + THROW_ERR_MEMORY_ALLOCATION_FAILED(env); 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()); } @@ -334,30 +303,18 @@ 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) { + THROW_ERR_MEMORY_ALLOCATION_FAILED(env); 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()); } @@ -403,24 +360,44 @@ 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); CHECK_NOT_NULL(env); // TODO(addaleax): Handle nullptr here. 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, @@ -1020,15 +997,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); } @@ -1055,7 +1030,8 @@ void SetupBufferJS(const FunctionCallbackInfo& args) { env->SetMethod(proto, "ucs2Write", StringWrite); env->SetMethod(proto, "utf8Write", StringWrite); - if (auto 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(); CHECK(args[1]->IsObject()); auto binding_object = args[1].As(); auto array_buffer = ArrayBuffer::New(env->isolate(), diff --git a/src/node_crypto.cc b/src/node_crypto.cc index e4f2c3103c8..c170f4ebca7 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -1556,7 +1556,6 @@ static void AddFingerprintDigest(const unsigned char* md, } } - static MaybeLocal ECPointToBuffer(Environment* env, const EC_GROUP* group, const EC_POINT* point, @@ -1567,16 +1566,20 @@ 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(); } - static Local X509ToObject(Environment* env, X509* cert) { EscapableHandleScope scope(env->isolate()); Local context = env->context(); @@ -1888,9 +1891,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()); } @@ -1911,9 +1914,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()); } @@ -1931,10 +1934,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()); } @@ -2352,11 +2355,12 @@ int SSLWrap::TLSExtStatusCallback(SSL* s, void* arg) { size_t len = Buffer::Length(obj); // OpenSSL takes control of the pointer after accepting it - char* data = node::Malloc(len); + auto* allocator = env->isolate()->GetArrayBufferAllocator(); + uint8_t* data = static_cast(allocator->AllocateUninitialized(len)); memcpy(data, resp, len); if (!SSL_set_tlsext_status_ocsp_resp(s, data, len)) - free(data); + allocator->Free(data, len); w->ocsp_response_.Reset(); return SSL_TLSEXT_ERR_OK; @@ -3036,11 +3040,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; @@ -3058,27 +3060,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(). @@ -3096,9 +3098,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()) { @@ -3106,15 +3107,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"); @@ -3122,11 +3122,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()); } @@ -3146,14 +3144,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(mode)) { @@ -3165,8 +3162,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, @@ -3195,33 +3201,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()); } @@ -3848,18 +3842,17 @@ void Verify::VerifyFinal(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(verify_result); } - template -bool PublicKeyCipher::Cipher(const char* key_pem, +bool PublicKeyCipher::Cipher(Environment* env, + const char* key_pem, int key_pem_len, const char* passphrase, int padding, const unsigned char* data, int len, - unsigned char** out, - size_t* out_len) { + AllocatedBuffer* out) { EVPKeyPointer pkey; // Check if this is a PKCS#8 or RSA public key before trying as X.509 and @@ -3890,14 +3883,21 @@ bool PublicKeyCipher::Cipher(const char* key_pem, 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; } @@ -3921,35 +3921,24 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo& args) { String::Utf8Value passphrase(args.GetIsolate(), args[3]); - unsigned char* out_value = nullptr; - size_t out_len = 0; + AllocatedBuffer out; ClearErrorOnReturn clear_error_on_return; bool r = Cipher( + env, kbuf, klen, args.Length() >= 4 && !args[3]->IsNull() ? *passphrase : nullptr, 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()); } @@ -4150,9 +4139,9 @@ void DiffieHellman::GenerateKeys(const FunctionCallbackInfo& args) { const BIGNUM* pub_key; DH_get0_key(diffieHellman->dh_.get(), &pub_key, nullptr); size_t size = BN_num_bytes(pub_key); - char* data = Malloc(size); - BN_bn2bin(pub_key, reinterpret_cast(data)); - args.GetReturnValue().Set(Buffer::New(env, data, size).ToLocalChecked()); + AllocatedBuffer data = env->AllocateManaged(size); + BN_bn2bin(pub_key, reinterpret_cast(data.data())); + args.GetReturnValue().Set(data.ToBuffer().ToLocalChecked()); } @@ -4169,9 +4158,9 @@ void DiffieHellman::GetField(const FunctionCallbackInfo& args, if (num == nullptr) return env->ThrowError(err_if_null); size_t size = BN_num_bytes(num); - char* data = Malloc(size); - BN_bn2bin(num, reinterpret_cast(data)); - args.GetReturnValue().Set(Buffer::New(env, data, size).ToLocalChecked()); + AllocatedBuffer data = env->AllocateManaged(size); + BN_bn2bin(num, reinterpret_cast(data.data())); + args.GetReturnValue().Set(data.ToBuffer().ToLocalChecked()); } void DiffieHellman::GetPrime(const FunctionCallbackInfo& args) { @@ -4233,9 +4222,9 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { Buffer::Length(args[0]), 0)); - 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()); @@ -4270,14 +4259,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 v8::FunctionCallbackInfo& args, @@ -4455,15 +4443,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); } @@ -4505,15 +4492,13 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo& args) { return env->ThrowError("Failed to get ECDH private key"); int size = BN_num_bytes(b); - unsigned char* out = node::Malloc(size); + AllocatedBuffer out = env->AllocateManaged(size); - if (size != BN_bn2bin(b, out)) { - free(out); + if (size != BN_bn2bin(b, reinterpret_cast(out.data()))) { return env->ThrowError("Failed to convert ECDH private key to Buffer"); } - Local buf = - Buffer::New(env, reinterpret_cast(out), size).ToLocalChecked(); + Local buf = out.ToBuffer().ToLocalChecked(); args.GetReturnValue().Set(buf); } @@ -4976,31 +4961,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; } @@ -5017,12 +4999,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 1a93ae7a47e..b9bff4adde4 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -383,9 +383,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; @@ -576,14 +575,14 @@ class PublicKeyCipher { template - static bool Cipher(const char* key_pem, + static bool Cipher(Environment* env, + const char* key_pem, int key_pem_len, const char* passphrase, 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); - IncrementCurrentSessionMemory(buf.len); CHECK(stream_buf_ab_.IsEmpty()); + AllocatedBuffer buf(env(), buf_); if (nread <= 0) { - free(buf.base); if (nread < 0) { PassReadErrorToPreviousListener(nread); } } else { // Only pass data on if nread > 0 + // Shrink to the actual amount of used data. + buf.Resize(nread); + + IncrementCurrentSessionMemory(buf.size()); + // Makre sure that there was no read previously active. CHECK_NULL(stream_buf_.base); CHECK_EQ(stream_buf_.len, 0); // 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(buf.base, nread); - - // Verify that currently: There is memory allocated into which - // the data has been read, and that memory buffer is at least as large - // as the amount of data we have read, but we have not yet made an - // ArrayBuffer out of it. - CHECK_LE(static_cast(nread), stream_buf_.len); + 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, - buf.base, - nread, - v8::ArrayBufferCreationMode::kInternalized); + stream_buf_ab_ = buf.ToArrayBuffer(); statistics_.data_received += nread; ssize_t ret = Write(&stream_buf_, 1); @@ -1737,7 +1735,7 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { // Since we are finished handling this write, reset the stream buffer. // The memory has either been free()d or was handed over to V8. - DecrementCurrentSessionMemory(buf.len); + DecrementCurrentSessionMemory(buf.size()); stream_buf_ab_ = Local(); stream_buf_ = uv_buf_init(nullptr, 0); diff --git a/src/node_http2.h b/src/node_http2.h index d7f8d9acae9..8ea6e5fc36f 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -779,6 +779,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.cc b/src/node_http_parser.cc index a4b92cbb20b..852f925b9bc 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -531,10 +531,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 fe446dbc713..3f66787fee8 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -416,10 +416,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); inline v8::MaybeLocal New(Environment* env, @@ -450,7 +452,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 20e0c7673b8..e8b569be772 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -118,6 +118,21 @@ MaybeLocal Message::Deserialize(Environment* env, // Attach all transfered ArrayBuffers to their new Isolate. for (uint32_t i = 0; i < array_buffer_contents_.size(); ++i) { + if (!env->isolate_data()->uses_node_allocator()) { + // We don't use Node's allocator on the receiving side, so we have + // to create the ArrayBuffer from a copy of the memory. + AllocatedBuffer buf = + env->AllocateManaged(array_buffer_contents_[i].size); + memcpy(buf.data(), + array_buffer_contents_[i].data, + array_buffer_contents_[i].size); + deserializer.TransferArrayBuffer(i, buf.ToArrayBuffer()); + 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(), @@ -266,6 +281,7 @@ Maybe Message::Serialize(Environment* env, // take ownership of its memory, copying the buffer will have to do. if (!ab->IsNeuterable() || ab->IsExternal()) continue; + } // We simply use the array index in the `array_buffers` list as the // ID that we write into the serialized buffer. uint32_t id = array_buffers.size(); @@ -311,6 +327,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_serdes.cc b/src/node_serdes.cc index 5de0ddd8190..79df651664e 100644 --- a/src/node_serdes.cc +++ b/src/node_serdes.cc @@ -205,10 +205,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 027b938d30d..21cf0d6d94b 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -412,18 +412,13 @@ 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 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 f429f3593fd..c8400f69833 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -127,9 +127,9 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { } } - std::unique_ptr storage; + AllocatedBuffer storage; if (storage_size > 0) - storage = std::unique_ptr(Malloc(storage_size)); + storage = env->AllocateManaged(storage_size); offset = 0; if (!all_buffers) { @@ -145,8 +145,8 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { // Write string CHECK_LE(offset, storage_size); - char* str_storage = storage.get() + 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(), @@ -164,8 +164,8 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { StreamWriteResult res = Write(*bufs, count, nullptr, req_wrap_obj); SetWriteResultPropertiesOnWrapObject(env, req_wrap_obj, res); - if (res.wrap != nullptr && storage) { - res.wrap->SetAllocatedStorage(storage.release(), storage_size); + if (res.wrap != nullptr && storage_size > 0) { + res.wrap->SetAllocatedStorage(std::move(storage)); } return res.err; } @@ -265,18 +265,18 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { CHECK_EQ(count, 1); } - std::unique_ptr data; + AllocatedBuffer data; if (try_write) { // Copy partial data - data = std::unique_ptr(Malloc(buf.len)); - memcpy(data.get(), buf.base, buf.len); + data = env->AllocateManaged(buf.len); + memcpy(data.data(), buf.base, buf.len); data_size = buf.len; } else { // Write it - data = std::unique_ptr(Malloc(storage_size)); + data = env->AllocateManaged(storage_size); data_size = StringBytes::Write(env->isolate(), - data.get(), + data.data(), storage_size, string, enc); @@ -284,7 +284,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { CHECK_LE(data_size, storage_size); - buf = uv_buf_init(data.get(), data_size); + buf = uv_buf_init(data.data(), data_size); uv_stream_t* send_handle = nullptr; @@ -302,7 +302,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { SetWriteResultPropertiesOnWrapObject(env, req_wrap_obj, res); if (res.wrap != nullptr) { - res.wrap->SetAllocatedStorage(data.release(), data_size); + res.wrap->SetAllocatedStorage(std::move(data)); } return res.err; @@ -356,31 +356,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 = Buffer::New(env, base, nread).ToLocalChecked(); - stream->CallJSOnreadMethod(nread, obj); + stream->CallJSOnreadMethod(nread, buf.ToBuffer().ToLocalChecked()); } diff --git a/src/stream_base.h b/src/stream_base.h index 05c2a962362..597ae0c9062 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -74,24 +74,18 @@ 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() { - 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 +109,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 +175,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 e19f98e35d2..2444b6edb8b 100644 --- a/src/stream_pipe.cc +++ b/src/stream_pipe.cc @@ -109,17 +109,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_); @@ -133,19 +133,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 c76afac4168..ea9ea853fed 100644 --- a/src/stream_pipe.h +++ b/src/stream_pipe.h @@ -44,7 +44,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 724f98c0cc9..b201a40e4c8 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -455,25 +455,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()); @@ -486,14 +484,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); }