FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http2: do not create ArrayBuffers when no DATA received · nodejs/node@7de642b · GitHub

/ node Public

Commit 7de642b

Browse files
addaleax
authored and
BethGriggs
committed
http2: do not create ArrayBuffers when no DATA received
Lazily allocate `ArrayBuffer`s for the contents of DATA frames. Creating `ArrayBuffer`s is, sadly, not a cheap operation with V8. This is part of performance improvements to mitigate CVE-2019-9513. Together with the previous commit, these changes improve throughput in the adversarial case by about 100 %, and there is little more that we can do besides artificially limiting the rate of incoming metadata frames (i.e. after this patch, CPU usage is virtually exclusively in libnghttp2). [This backport also applies changes from 83e1b97 and required some manual work due to the lack of `AllocatedBuffer` on v10.x. More work was necessary for v8.x, including copying utilities for `util.h` from more recent Node.js versions.] Refs: #26201 Backport-PR-URL: #29124 PR-URL: #29122 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent dd60d35 commit 7de642b

4 files changed

Lines changed: 92 additions & 71 deletions

File tree

‎src/node_http2.cc‎

Lines changed: 76 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,7 @@ Http2Session::Http2Session(Environment* env,
675675
ArrayBuffer::New(env->isolate(), js_fields_, kSessionUint8FieldCount);
676676
Local<Uint8Array> uint8_arr =
677677
Uint8Array::New(ab, 0, kSessionUint8FieldCount);
678-
/*USE*/(wrap->Set(env->context(), env->fields_string(), uint8_arr));
678+
USE(wrap->Set(env->context(), env->fields_string(), uint8_arr));
679679
}
680680
}
681681

@@ -702,6 +702,7 @@ Http2Session::~Http2Session() {
702702
DEBUG_HTTP2SESSION(this, "freeing nghttp2 session");
703703
nghttp2_session_del(session_);
704704
CHECK_EQ(current_nghttp2_memory_, 0);
705+
free(stream_buf_allocation_.base);
705706
}
706707

707708
inline bool HasHttp2Observer(Environment* env) {
@@ -1211,18 +1212,31 @@ inline int Http2Session::OnDataChunkReceived(nghttp2_session* handle,
12111212

12121213
stream->statistics_.received_bytes += len;
12131214

1215+
Local<ArrayBuffer> ab;
1216+
if (session->stream_buf_ab_.IsEmpty()) {
1217+
ab = ArrayBuffer::New(env->isolate(),
1218+
session->stream_buf_allocation_.base,
1219+
session->stream_buf_allocation_.len,
1220+
v8::ArrayBufferCreationMode::kInternalized);
1221+
session->stream_buf_allocation_ = uv_buf_init(nullptr, 0);
1222+
session->stream_buf_ab_.Reset(env->isolate(), ab);
1223+
} else {
1224+
ab = session->stream_buf_ab_.Get(env->isolate());
1225+
}
1226+
12141227
// There is a single large array buffer for the entire data read from the
12151228
// network; create a slice of that array buffer and emit it as the
12161229
// received data buffer.
1217-
CHECK(!session->stream_buf_ab_.IsEmpty());
1218-
size_t offset = reinterpret_cast<const char*>(data) - session->stream_buf_;
1230+
size_t offset = data - reinterpret_cast<uint8_t*>(session->stream_buf_.base);
1231+
12191232
// Verify that the data offset is inside the current read buffer.
1220-
CHECK_LE(offset, session->stream_buf_size_);
1233+
CHECK_LE(offset, session->stream_buf_.len);
1234+
CHECK_LE(offset + len, session->stream_buf_.len);
12211235

1222-
Local<Object> buf =
1223-
Buffer::New(env, session->stream_buf_ab_, offset, len).ToLocalChecked();
1236+
Local<Object> buffer =
1237+
Buffer::New(env, ab, offset, len).ToLocalChecked();
12241238

1225-
stream->EmitData(len, buf, Local<Object>());
1239+
stream->EmitData(len, buffer, Local<Object>());
12261240
if (!stream->IsReading())
12271241
stream->inbound_consumed_data_while_paused_ += len;
12281242
else
@@ -1841,83 +1855,82 @@ void Http2Session::OnStreamAllocImpl(size_t suggested_size,
18411855
uv_buf_t* buf,
18421856
void* ctx) {
18431857
Http2Session* session = static_cast<Http2Session*>(ctx);
1844-
CHECK_EQ(session->stream_buf_, nullptr);
1845-
CHECK_EQ(session->stream_buf_size_, 0);
1846-
buf->base = session->stream_buf_ = Malloc(suggested_size);
1847-
buf->len = session->stream_buf_size_ = suggested_size;
1848-
session->IncrementCurrentSessionMemory(suggested_size);
1858+
CHECK_EQ(session->stream_buf_.base, nullptr);
1859+
CHECK_EQ(session->stream_buf_.len, 0);
1860+
*buf = uv_buf_init(Malloc(suggested_size), suggested_size);
18491861
}
18501862

18511863
// Callback used to receive inbound data from the i/o stream
18521864
void Http2Session::OnStreamReadImpl(ssize_t nread,
1853-
const uv_buf_t* buf,
1865+
const uv_buf_t* buf_,
18541866
uv_handle_type pending,
18551867
void* ctx) {
18561868
Http2Session* session = static_cast<Http2Session*>(ctx);
18571869
Http2Scope h2scope(session);
18581870
CHECK_NE(session->stream_, nullptr);
18591871
DEBUG_HTTP2SESSION2(session, "receiving %d bytes", nread);
1872+
CHECK_EQ(session->stream_buf_allocation_.base, nullptr);
1873+
CHECK(session->stream_buf_ab_.IsEmpty());
1874+
1875+
// Only pass data on if nread > 0
18601876
if (nread <= 0) {
1861-
free(session->stream_buf_);
1877+
if (buf_ != nullptr)
1878+
free(buf_->base);
18621879
if (nread < 0) {
18631880
uv_buf_t tmp_buf = uv_buf_init(nullptr, 0);
18641881
session->prev_read_cb_.fn(nread,
18651882
&tmp_buf,
18661883
pending,
18671884
session->prev_read_cb_.ctx);
18681885
}
1869-
} else {
1870-
// Only pass data on if nread > 0
1871-
1872-
// Verify that currently: There is memory allocated into which
1873-
// the data has been read, and that memory buffer is at least as large
1874-
// as the amount of data we have read, but we have not yet made an
1875-
// ArrayBuffer out of it.
1876-
CHECK_NE(session->stream_buf_, nullptr);
1877-
CHECK_EQ(session->stream_buf_, buf->base);
1878-
CHECK_EQ(session->stream_buf_size_, buf->len);
1879-
CHECK_GE(session->stream_buf_size_, static_cast<size_t>(nread));
1880-
CHECK(session->stream_buf_ab_.IsEmpty());
1886+
return;
1887+
}
18811888

1882-
Environment* env = session->env();
1883-
Isolate* isolate = env->isolate();
1884-
HandleScope scope(isolate);
1885-
Local<Context> context = env->context();
1886-
Context::Scope context_scope(context);
1889+
// Shrink to the actual amount of used data.
1890+
uv_buf_t buf = *buf_;
1891+
buf.base = Realloc(buf.base, nread);
1892+
1893+
session->IncrementCurrentSessionMemory(nread);
1894+
OnScopeLeave on_scope_leave([&]() {
1895+
// Once finished handling this write, reset the stream buffer.
1896+
// The memory has either been free()d or was handed over to V8.
1897+
// We use `nread` instead of `buf.size()` here, because the buffer is
1898+
// cleared as part of the `.ToArrayBuffer()` call below.
1899+
session->DecrementCurrentSessionMemory(nread);
1900+
session->stream_buf_ab_.Reset();
1901+
free(session->stream_buf_allocation_.base);
1902+
session->stream_buf_allocation_ = uv_buf_init(nullptr, 0);
1903+
session->stream_buf_ = uv_buf_init(nullptr, 0);
1904+
});
18871905

1888-
// Create an array buffer for the read data. DATA frames will be emitted
1889-
// as slices of this array buffer to avoid having to copy memory.
1890-
session->stream_buf_ab_ =
1891-
ArrayBuffer::New(isolate,
1892-
session->stream_buf_,
1893-
session->stream_buf_size_,
1894-
v8::ArrayBufferCreationMode::kInternalized);
1895-
1896-
uv_buf_t buf_ = uv_buf_init(buf->base, nread);
1897-
session->statistics_.data_received += nread;
1898-
ssize_t ret = session->Write(&buf_, 1);
1899-
1900-
// Note: if ssize_t is not defined (e.g. on Win32), nghttp2 will typedef
1901-
// ssize_t to int. Cast here so that the < 0 check actually works on
1902-
// Windows.
1903-
if (static_cast<int>(ret) < 0) {
1904-
DEBUG_HTTP2SESSION2(session, "fatal error receiving data: %d", ret);
1905-
1906-
Local<Value> argv[1] = {
1907-
Integer::New(isolate, ret),
1908-
};
1909-
session->MakeCallback(env->error_string(), arraysize(argv), argv);
1910-
} else {
1911-
session->MaybeStopReading();
1912-
}
1913-
}
1906+
// Make sure that there was no read previously active.
1907+
CHECK_EQ(session->stream_buf_.base, nullptr);
1908+
CHECK_EQ(session->stream_buf_.len, 0);
1909+
1910+
// Remember the current buffer, so that OnDataChunkReceived knows the
1911+
// offset of a DATA frame's data into the socket read buffer.
1912+
session->stream_buf_ = uv_buf_init(buf.base, nread);
19141913

1915-
// Since we are finished handling this write, reset the stream buffer.
1916-
// The memory has either been free()d or was handed over to V8.
1917-
session->DecrementCurrentSessionMemory(session->stream_buf_size_);
1918-
session->stream_buf_ = nullptr;
1919-
session->stream_buf_size_ = 0;
1920-
session->stream_buf_ab_ = Local<ArrayBuffer>();
1914+
// Verify that currently: There is memory allocated into which
1915+
// the data has been read, and that memory buffer is at least as large
1916+
// as the amount of data we have read, but we have not yet made an
1917+
// ArrayBuffer out of it.
1918+
CHECK_LE(static_cast<size_t>(nread), session->stream_buf_.len);
1919+
1920+
// Store this so we can create an ArrayBuffer for read data from it.
1921+
// DATA frames will be emitted as slices of that ArrayBuffer to avoid having
1922+
// to copy memory.
1923+
session->stream_buf_allocation_ = buf;
1924+
1925+
session->statistics_.data_received += nread;
1926+
ssize_t ret = session->Write(&session->stream_buf_, 1);
1927+
1928+
if (UNLIKELY(ret < 0)) {
1929+
DEBUG_HTTP2SESSION2(session, "fatal error receiving data: %d", ret);
1930+
Local<Value> arg = Integer::New(session->env()->isolate(), ret);
1931+
session->MakeCallback(session->env()->error_string(), 1, &arg);
1932+
return;
1933+
}
19211934
}
19221935

19231936
void Http2Session::OnStreamDestructImpl(void* ctx) {

‎src/node_http2.h‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -834,10 +834,6 @@ class Http2Session : public AsyncWrap {
834834

835835
size_t self_size() const override { return sizeof(*this); }
836836

837-
char* stream_alloc() {
838-
return stream_buf_;
839-
}
840-
841837
// Schedule an RstStream for after the current write finishes.
842838
inline void AddPendingRstStream(int32_t stream_id) {
843839
pending_rst_streams_.emplace_back(stream_id);
@@ -1062,9 +1058,9 @@ class Http2Session : public AsyncWrap {
10621058
// use this to allow timeout tracking during long-lasting writes
10631059
uint32_t chunks_sent_since_last_write_ = 0;
10641060

1065-
char* stream_buf_ = nullptr;
1066-
size_t stream_buf_size_ = 0;
1067-
v8::Local<v8::ArrayBuffer> stream_buf_ab_;
1061+
uv_buf_t stream_buf_ = uv_buf_init(nullptr, 0);
1062+
v8::Global<v8::ArrayBuffer> stream_buf_ab_;
1063+
uv_buf_t stream_buf_allocation_ = uv_buf_init(nullptr, 0);
10681064

10691065
size_t max_outstanding_pings_ = DEFAULT_MAX_PINGS;
10701066
std::queue<Http2Ping*> outstanding_pings_;

‎src/util.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include <stdlib.h>
3434
#include <string.h>
3535

36+
#include <functional>
3637
#include <type_traits> // std::remove_reference
3738

3839
namespace node {
@@ -433,6 +434,17 @@ class BufferValue : public MaybeStackBuffer<char> {
433434
if (name##_length > 0) \
434435
CHECK_NE(name##_data, nullptr);
435436

437+
// Use this when a variable or parameter is unused in order to explicitly
438+
// silence a compiler warning about that.
439+
template <typename T> inline void USE(T&&) {}
440+
441+
// Run a function when exiting the current scope.
442+
struct OnScopeLeave {
443+
std::function<void()> fn_;
444+
445+
explicit OnScopeLeave(std::function<void()> fn) : fn_(fn) {}
446+
~OnScopeLeave() { fn_(); }
447+
};
436448

437449
} // namespace node
438450

‎test/sequential/test-http2-max-session-memory.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const http2 = require('http2');
88

99
// Test that maxSessionMemory Caps work
1010

11-
const largeBuffer = Buffer.alloc(1e6);
11+
const largeBuffer = Buffer.alloc(2e6);
1212

1313
const server = http2.createServer({ maxSessionMemory: 1 });
1414

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL