You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "wwbmmm (via GitHub)" <gi...@apache.org> on 2026/08/15 08:37:43 UTC
[PR] Fix RTMP abort message deleting the chunk stream being parsed (brpc)
wwbmmm opened a new pull request, #3452:
URL: https://github.com/apache/brpc/pull/3452
### What problem does this PR solve?
Issue Number: null
Problem Summary:
An RTMP Abort control message that references the chunk stream carrying
the message itself made the parser free that chunk stream while it was
still being processed. `RtmpChunkStream::Feed()` continues to access the
freed chunk stream right after `OnMessage()` returns, which may crash the
server or corrupt memory.
### What is changed and the side effects?
Changed:
- `RtmpChunkStream::OnAbortMessage` now skips `ClearChunkStream` when the
abort targets the chunk stream that is currently being parsed.
- Added a unit test that feeds a self-referencing Abort message and
verifies the chunk stream survives.
Side effects:
- Performance effects: None.
- Breaking backward compatibility: None.
---
### Check List:
- Please make sure your changes are compilable.
- When providing us with a new feature, it is best to add related tests.
- Please follow [Contributor Covenant Code of Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix RTMP abort message deleting the chunk stream being parsed (brpc)
Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright merged PR #3452:
URL: https://github.com/apache/brpc/pull/3452
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix RTMP abort message deleting the chunk stream being parsed (brpc)
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3452:
URL: https://github.com/apache/brpc/pull/3452#discussion_r3788975571
##########
test/brpc_rtmp_unittest.cpp:
##########
@@ -805,6 +807,52 @@ TEST(RtmpTest, flv_reader_rejects_zero_datasize_audio_tag) {
ASSERT_EQ(before, buf.size());
}
+// A crafted Abort message that names the chunk stream currently being parsed
+// (itself) used to make ClearChunkStream delete the RtmpChunkStream while its
+// Feed() is still running, which caused a heap-use-after-free right after
+// OnMessage() returned.
+TEST(RtmpTest, abort_message_naming_own_chunk_stream) {
+ int pipe_fds[2];
+ ASSERT_EQ(0, pipe(pipe_fds));
+ butil::fd_guard guard0(pipe_fds[0]);
+ butil::fd_guard guard1(pipe_fds[1]);
+
+ brpc::SocketId id;
+ brpc::SocketOptions options;
+ options.fd = pipe_fds[1];
+ ASSERT_EQ(0, brpc::Socket::Create(options, &id));
Review Comment:
`SocketOptions::fd` takes ownership of the fd and `Socket` will close it on recycle/destruction. Keeping `fd_guard guard1(pipe_fds[1])` causes a double-close (and can accidentally close an unrelated fd if the descriptor number gets reused). Transfer ownership to `Socket` by releasing the guard when assigning `options.fd`.
##########
test/brpc_rtmp_unittest.cpp:
##########
@@ -805,6 +807,52 @@ TEST(RtmpTest, flv_reader_rejects_zero_datasize_audio_tag) {
ASSERT_EQ(before, buf.size());
}
+// A crafted Abort message that names the chunk stream currently being parsed
+// (itself) used to make ClearChunkStream delete the RtmpChunkStream while its
+// Feed() is still running, which caused a heap-use-after-free right after
+// OnMessage() returned.
+TEST(RtmpTest, abort_message_naming_own_chunk_stream) {
+ int pipe_fds[2];
+ ASSERT_EQ(0, pipe(pipe_fds));
+ butil::fd_guard guard0(pipe_fds[0]);
+ butil::fd_guard guard1(pipe_fds[1]);
+
+ brpc::SocketId id;
+ brpc::SocketOptions options;
+ options.fd = pipe_fds[1];
+ ASSERT_EQ(0, brpc::Socket::Create(options, &id));
+ brpc::SocketUniquePtr sock;
+ ASSERT_EQ(0, brpc::Socket::Address(id, &sock));
+
+ brpc::policy::RtmpContext ctx(NULL, NULL);
+ ctx.SetState(sock->remote_side(),
+ brpc::policy::RtmpContext::STATE_RECEIVED_C2);
+
+ // fmt0 chunk on chunk stream 2 carrying an Abort message (type 2) whose
+ // payload is the same chunk stream id (2).
+ std::string chunk;
+ chunk.push_back((char)0x02); // basic header: fmt=0, cs_id=2
+ chunk.append(3, '\0'); // timestamp = 0
+ chunk.push_back('\0'); // message_length (3 bytes) = 4
+ chunk.push_back('\0');
+ chunk.push_back((char)0x04);
+ chunk.push_back((char)0x02); // message_type = Abort
+ chunk.append(4, '\0'); // stream_id = 0 (little endian)
+ chunk.push_back('\0'); // payload: cs_id = 2 (big endian)
+ chunk.push_back('\0');
+ chunk.push_back('\0');
+ chunk.push_back((char)0x02);
+
+ butil::IOBuf buf;
+ buf.append(chunk);
+ const brpc::ParseResult pr = ctx.Feed(&buf, sock.get());
+ ASSERT_EQ(brpc::PARSE_OK, pr.error());
+
+ // The chunk stream being parsed must survive the abort message: it must
+ // not be deleted by ClearChunkStream while Feed() is still running.
+ ASSERT_TRUE(ctx.GetChunkStream(2) != NULL);
Review Comment:
`ASSERT_TRUE(ctx.GetChunkStream(2) != NULL)` is effectively tautological: for a valid id, `GetChunkStream` will create and return a chunk stream even if it was just deleted, so this doesn't actually verify the stream “survived” the Abort message. Capture the chunk stream pointer before feeding and assert that the same instance remains after `Feed()` returns.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org