1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

http2: do not create ArrayBuffers when no DATA received

Browse Source 
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).

PR-URL: https://github.com/nodejs/node/pull/29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Anna Henningsen 2019-08-10 01:30:35 +02:00 committed by Michaël Zasso
parent c44ee7a14a
commit 599eee0990
No known key found for this signature in database
GPG Key ID: 770F7A9A5AE15600
2 changed files with 17 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/node_http2.cc
View File

@ -1259,7 +1259,13 @@ void Http2StreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) {
return; return;
} }
CHECK(!session->stream_buf_ab_.IsEmpty()); Local<ArrayBuffer> ab;
if (session->stream_buf_ab_.IsEmpty()) {
ab = session->stream_buf_allocation_.ToArrayBuffer();
session->stream_buf_ab_.Reset(env->isolate(), ab);
} else {
ab = PersistentToLocal::Strong(session->stream_buf_ab_);
}
// There is a single large array buffer for the entire data read from the // There is a single large array buffer for the entire data read from the
// network; create a slice of that array buffer and emit it as the // network; create a slice of that array buffer and emit it as the
@ -1270,7 +1276,7 @@ void Http2StreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) {
CHECK_LE(offset, session->stream_buf_.len); CHECK_LE(offset, session->stream_buf_.len);
CHECK_LE(offset + buf.len, session->stream_buf_.len); CHECK_LE(offset + buf.len, session->stream_buf_.len);
stream->CallJSOnreadMethod(nread, session->stream_buf_ab_, offset); stream->CallJSOnreadMethod(nread, ab, offset);
} }
@ -1789,6 +1795,7 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
Http2Scope h2scope(this); Http2Scope h2scope(this);
CHECK_NOT_NULL(stream_); CHECK_NOT_NULL(stream_);
Debug(this, "receiving %d bytes", nread); Debug(this, "receiving %d bytes", nread);
CHECK_EQ(stream_buf_allocation_.size(), 0);
CHECK(stream_buf_ab_.IsEmpty()); CHECK(stream_buf_ab_.IsEmpty());
AllocatedBuffer buf(env(), buf_); AllocatedBuffer buf(env(), buf_);
@ -1810,7 +1817,8 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
// We use `nread` instead of `buf.size()` here, because the buffer is // We use `nread` instead of `buf.size()` here, because the buffer is
// cleared as part of the `.ToArrayBuffer()` call below. // cleared as part of the `.ToArrayBuffer()` call below.
DecrementCurrentSessionMemory(nread); DecrementCurrentSessionMemory(nread);
stream_buf_ab_ = Local<ArrayBuffer>(); stream_buf_ab_.Reset();
stream_buf_allocation_.clear();
stream_buf_ = uv_buf_init(nullptr, 0); stream_buf_ = uv_buf_init(nullptr, 0);
}); });
@ -1824,9 +1832,10 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
Isolate* isolate = env()->isolate(); Isolate* isolate = env()->isolate();
// Create an array buffer for the read data. DATA frames will be emitted // Store this so we can create an ArrayBuffer for read data from it.
// as slices of this array buffer to avoid having to copy memory. // DATA frames will be emitted as slices of that ArrayBuffer to avoid having
stream_buf_ab_ = buf.ToArrayBuffer(); // to copy memory.
stream_buf_allocation_ = std::move(buf);
statistics_.data_received += nread; statistics_.data_received += nread;
ssize_t ret = Write(&stream_buf_, 1); ssize_t ret = Write(&stream_buf_, 1);

3
src/node_http2.h
View File

@ -993,7 +993,8 @@ class Http2Session : public AsyncWrap, public StreamListener {
uint32_t chunks_sent_since_last_write_ = 0; uint32_t chunks_sent_since_last_write_ = 0;
uv_buf_t stream_buf_ = uv_buf_init(nullptr, 0); uv_buf_t stream_buf_ = uv_buf_init(nullptr, 0);
v8::Local<v8::ArrayBuffer> stream_buf_ab_; v8::Global<v8::ArrayBuffer> stream_buf_ab_;
AllocatedBuffer stream_buf_allocation_;
size_t max_outstanding_pings_ = DEFAULT_MAX_PINGS; size_t max_outstanding_pings_ = DEFAULT_MAX_PINGS;
std::queue<std::unique_ptr<Http2Ping>> outstanding_pings_; std::queue<std::unique_ptr<Http2Ping>> outstanding_pings_;