Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v12.x] http2: patch double-free error due to handling of rst_stream #39527

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2150,6 +2150,23 @@ int Http2Stream::SubmitPriority(nghttp2_priority_spec* prispec,
void Http2Stream::SubmitRstStream(const uint32_t code) {
CHECK(!this->IsDestroyed());
code_ = code;

// If RST_STREAM frame is received and stream is not writable
// because it is busy reading data, don't try force purging it.
// Instead add the stream to pending stream list and process
// the pending data when it is safe to do so. This is to avoid
// double free error due to unwanted behavior of nghttp2.
// Ref:https://github.com/nodejs/node/issues/38964

// Add stream to the pending list if it is received with scope
// below in the stack. The pending list may not get processed
// if RST_STREAM received is not in scope and added to the list
// causing endpoint to hang.
if (session_->is_in_scope() && IsReading()) {
session_->AddPendingRstStream(id_);
return;
}

// If possible, force a purge of any currently pending data here to make sure
// it is sent before closing the stream. If it returns non-zero then we need
// to wait until the current write finishes and try again to avoid nghttp2
Expand Down
16 changes: 16 additions & 0 deletions src/node_http2.h
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,22 @@ class Http2Session : public AsyncWrap,
return (flags_ & SESSION_STATE_CLOSED) || session_ == nullptr;
}


// The changes are backported and exposes APIs to check the
// status flag of `Http2Session`
#define IS_FLAG(name, flag) \
bool is_##name() const { return flags_ & flag; }

IS_FLAG(in_scope, SESSION_STATE_HAS_SCOPE)
IS_FLAG(write_scheduled, SESSION_STATE_WRITE_SCHEDULED)
IS_FLAG(closing, SESSION_STATE_CLOSING)
IS_FLAG(sending, SESSION_STATE_SENDING)
IS_FLAG(write_in_progress, SESSION_STATE_WRITE_IN_PROGRESS)
IS_FLAG(reading_stopped, SESSION_STATE_READING_STOPPED)
IS_FLAG(receive_paused, SESSION_STATE_NGHTTP2_RECV_PAUSED)

#undef IS_FLAG

// Schedule a write if nghttp2 indicates it wants to write to the socket.
void MaybeScheduleWrite();

Expand Down