RTC: Fix null pointer crash in RTC2RTMP when start packet is missing. v6.0.175 (#4451)
Try to fix #4450 ## Cause The SRS transcode rtp packets, whose sequence number in range [start, end], to one rtmp packet, but when the first rtp packet is empty, then this crash happens. check #4450 for details. ## Impact 5.0release and 6.0release branch. develop branch already has its own solution. So this PR is targeting to **6.0release**. ## Solution find the first not empty rtp packet in seq range [start, end]. --------- Co-authored-by: OSSRS-AI <winlinam@gmail.com> Co-authored-by: winlin <winlinvip@gmail.com>
This commit is contained in:
parent
2ab3937a68
commit
c1a8a5f753
|
|
@ -671,6 +671,11 @@ fi
|
||||||
if [[ $SRS_SRT == YES && $SRS_USE_SYS_SRT == NO ]]; then
|
if [[ $SRS_SRT == YES && $SRS_USE_SYS_SRT == NO ]]; then
|
||||||
# Always disable c++11 for libsrt, because only the srt-app requres it.
|
# Always disable c++11 for libsrt, because only the srt-app requres it.
|
||||||
LIBSRT_OPTIONS="--enable-apps=0 --enable-static=1 --enable-c++11=0"
|
LIBSRT_OPTIONS="--enable-apps=0 --enable-static=1 --enable-c++11=0"
|
||||||
|
CMAKE_VERSION=$(cmake --version | head -n1 | cut -d' ' -f3)
|
||||||
|
CMAKE_MAJOR=$(echo $CMAKE_VERSION | cut -d'.' -f1)
|
||||||
|
if [[ $CMAKE_MAJOR -ge 4 ]]; then
|
||||||
|
LIBSRT_OPTIONS="$LIBSRT_OPTIONS --CMAKE_POLICY_VERSION_MINIMUM=3.5"
|
||||||
|
fi
|
||||||
if [[ $SRS_SHARED_SRT == YES ]]; then
|
if [[ $SRS_SHARED_SRT == YES ]]; then
|
||||||
LIBSRT_OPTIONS="$LIBSRT_OPTIONS --enable-shared=1"
|
LIBSRT_OPTIONS="$LIBSRT_OPTIONS --enable-shared=1"
|
||||||
else
|
else
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,7 @@ The changelog for SRS.
|
||||||
<a name="v6-changes"></a>
|
<a name="v6-changes"></a>
|
||||||
|
|
||||||
## SRS 6.0 Changelog
|
## SRS 6.0 Changelog
|
||||||
|
* v6.0, 2025-08-26, Merge [#4451](https://github.com/ossrs/srs/pull/4451): RTC: Fix null pointer crash in RTC2RTMP when start packet is missing. v6.0.175 (#4451)
|
||||||
* v6.0, 2025-08-16, Merge [#4441](https://github.com/ossrs/srs/pull/4441): fix err memory leak in rtc to rtmp bridge. v6.0.174 (#4441)
|
* v6.0, 2025-08-16, Merge [#4441](https://github.com/ossrs/srs/pull/4441): fix err memory leak in rtc to rtmp bridge. v6.0.174 (#4441)
|
||||||
* v6.0, 2025-08-14, Merge [#4161](https://github.com/ossrs/srs/pull/4161): fix hls & dash segments cleanup. v6.0.173 (#4161)
|
* v6.0, 2025-08-14, Merge [#4161](https://github.com/ossrs/srs/pull/4161): fix hls & dash segments cleanup. v6.0.173 (#4161)
|
||||||
* v6.0, 2025-08-12, Merge [#4230](https://github.com/ossrs/srs/pull/4230): MP4 DVR: Fix audio/video synchronization issues in WebRTC recordings. v6.0.172 (#4230)
|
* v6.0, 2025-08-12, Merge [#4230](https://github.com/ossrs/srs/pull/4230): MP4 DVR: Fix audio/video synchronization issues in WebRTC recordings. v6.0.172 (#4230)
|
||||||
|
|
|
||||||
|
|
@ -1798,13 +1798,26 @@ srs_error_t SrsRtcFrameBuilder::packet_video_rtmp(const uint16_t start, const ui
|
||||||
//type_codec1 + avc_type + composition time + nalu size + nalu
|
//type_codec1 + avc_type + composition time + nalu size + nalu
|
||||||
nb_payload += 1 + 1 + 3;
|
nb_payload += 1 + 1 + 3;
|
||||||
|
|
||||||
|
// find the first not emtpy rtp packet, which is always exist in seq range [start, end],
|
||||||
|
// because nb_payload check above.
|
||||||
|
// @see https://github.com/ossrs/srs/issues/4450
|
||||||
|
SrsRtpPacket* first_pkt = NULL;
|
||||||
|
for (uint16_t i = 0; i < (uint16_t)cnt; ++i) {
|
||||||
|
uint16_t index = cache_index(start + i);
|
||||||
|
SrsRtpPacket* pkt = cache_video_pkts_[index].pkt;
|
||||||
|
|
||||||
|
if (pkt) {
|
||||||
|
first_pkt = pkt;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
SrsCommonMessage rtmp;
|
SrsCommonMessage rtmp;
|
||||||
SrsRtpPacket* pkt = cache_video_pkts_[cache_index(start)].pkt;
|
rtmp.header.initialize_video(nb_payload, first_pkt->get_avsync_time(), 1);
|
||||||
rtmp.header.initialize_video(nb_payload, pkt->get_avsync_time(), 1);
|
|
||||||
rtmp.create_payload(nb_payload);
|
rtmp.create_payload(nb_payload);
|
||||||
rtmp.size = nb_payload;
|
rtmp.size = nb_payload;
|
||||||
SrsBuffer payload(rtmp.payload, rtmp.size);
|
SrsBuffer payload(rtmp.payload, rtmp.size);
|
||||||
if (pkt->is_keyframe()) {
|
if (first_pkt->is_keyframe()) {
|
||||||
payload.write_1bytes(0x17); // type(4 bits): key frame; code(4bits): avc
|
payload.write_1bytes(0x17); // type(4 bits): key frame; code(4bits): avc
|
||||||
rtp_key_frame_ts_ = -1;
|
rtp_key_frame_ts_ = -1;
|
||||||
} else {
|
} else {
|
||||||
|
|
|
||||||
|
|
@ -9,6 +9,6 @@
|
||||||
|
|
||||||
#define VERSION_MAJOR 6
|
#define VERSION_MAJOR 6
|
||||||
#define VERSION_MINOR 0
|
#define VERSION_MINOR 0
|
||||||
#define VERSION_REVISION 174
|
#define VERSION_REVISION 175
|
||||||
|
|
||||||
#endif
|
#endif
|
||||||
|
|
|
||||||
|
|
@ -1365,3 +1365,124 @@ VOID TEST(KernelRTCTest, JitterSequence)
|
||||||
EXPECT_EQ((uint32_t)11, jitter.correct(11));
|
EXPECT_EQ((uint32_t)11, jitter.correct(11));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Helper function to create a test RTP packet
|
||||||
|
SrsRtpPacket *mock_create_test_rtp_packet(uint16_t sequence_number, uint32_t timestamp, bool marker = false)
|
||||||
|
{
|
||||||
|
SrsRtpPacket *pkt = new SrsRtpPacket();
|
||||||
|
pkt->header.set_sequence(sequence_number);
|
||||||
|
pkt->header.set_timestamp(timestamp);
|
||||||
|
pkt->header.set_marker(marker);
|
||||||
|
pkt->header.set_ssrc(12345);
|
||||||
|
return pkt;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Mock bridge for testing SrsRtcFrameBuilder
|
||||||
|
class MockStreamBridge : public ISrsStreamBridge
|
||||||
|
{
|
||||||
|
public:
|
||||||
|
srs_error_t last_error;
|
||||||
|
int frame_count;
|
||||||
|
|
||||||
|
MockStreamBridge() {
|
||||||
|
last_error = srs_success;
|
||||||
|
frame_count = 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
virtual ~MockStreamBridge() {
|
||||||
|
srs_freep(last_error);
|
||||||
|
}
|
||||||
|
|
||||||
|
virtual srs_error_t initialize(SrsRequest *r) {
|
||||||
|
return srs_success;
|
||||||
|
}
|
||||||
|
|
||||||
|
virtual srs_error_t on_publish() {
|
||||||
|
return srs_success;
|
||||||
|
}
|
||||||
|
|
||||||
|
virtual srs_error_t on_frame(SrsSharedPtrMessage *frame) {
|
||||||
|
frame_count++;
|
||||||
|
return srs_success;
|
||||||
|
}
|
||||||
|
|
||||||
|
virtual void on_unpublish() {
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
VOID TEST(KernelRTC2Test, SrsRtcFrameBuilderPacketVideoRtmpNullPointerCrash)
|
||||||
|
{
|
||||||
|
srs_error_t err;
|
||||||
|
|
||||||
|
// Test reproducing the null pointer crash from issue #4450 fixed by PR #4451
|
||||||
|
//
|
||||||
|
// ISSUE BACKGROUND:
|
||||||
|
// Before PR 4451, the packet_video_rtmp() function assumed that the packet at the
|
||||||
|
// 'start' sequence number would always be available in the cache. However, due to
|
||||||
|
// network packet loss or reordering, the packet at the start position could be missing.
|
||||||
|
//
|
||||||
|
// THE CRASH:
|
||||||
|
// The original code did: pkt = cache_video_pkts_[cache_index(start)].pkt;
|
||||||
|
// When pkt was NULL, calling pkt->get_avsync_time() caused a segmentation fault.
|
||||||
|
//
|
||||||
|
// THE FIX:
|
||||||
|
// PR #4451 added a loop to find the first non-null packet in the sequence range
|
||||||
|
// instead of blindly using the packet at the start position.
|
||||||
|
//
|
||||||
|
// This test simulates the crash scenario: packets exist at positions 101, 102, 103
|
||||||
|
// but the start packet (100) is missing. With the fix, the function should use
|
||||||
|
// packet 101 (first available) instead of crashing on the missing packet 100.
|
||||||
|
if (true) {
|
||||||
|
MockStreamBridge bridge;
|
||||||
|
SrsRtcFrameBuilder frame_builder(&bridge);
|
||||||
|
|
||||||
|
// Skip initialization and directly set up the test scenario
|
||||||
|
// We only need to test the packet_video_rtmp function, not the full initialization
|
||||||
|
|
||||||
|
// Manually populate the video cache to simulate the crash scenario
|
||||||
|
// We'll store packets at positions 101, 102, 103 but NOT at position 100 (start)
|
||||||
|
// This simulates network packet loss where the first packet is missing
|
||||||
|
|
||||||
|
// Create test packets with payload to ensure nb_payload > 0
|
||||||
|
SrsRtpPacket *pkt101 = mock_create_test_rtp_packet(101, 1000, false);
|
||||||
|
SrsRtpPacket *pkt102 = mock_create_test_rtp_packet(102, 1000, false);
|
||||||
|
SrsRtpPacket *pkt103 = mock_create_test_rtp_packet(103, 1000, true); // marker bit
|
||||||
|
|
||||||
|
// Add some payload to ensure packets are not empty
|
||||||
|
char payload_data[] = "test_payload_data";
|
||||||
|
SrsRtpRawPayload *payload101 = new SrsRtpRawPayload();
|
||||||
|
payload101->payload = (char*)payload_data;
|
||||||
|
payload101->nn_payload = strlen(payload_data);
|
||||||
|
pkt101->set_payload(payload101, SrsRtspPacketPayloadTypeRaw);
|
||||||
|
|
||||||
|
SrsRtpRawPayload *payload102 = new SrsRtpRawPayload();
|
||||||
|
payload102->payload = (char*)payload_data;
|
||||||
|
payload102->nn_payload = strlen(payload_data);
|
||||||
|
pkt102->set_payload(payload102, SrsRtspPacketPayloadTypeRaw);
|
||||||
|
|
||||||
|
SrsRtpRawPayload *payload103 = new SrsRtpRawPayload();
|
||||||
|
payload103->payload = (char*)payload_data;
|
||||||
|
payload103->nn_payload = strlen(payload_data);
|
||||||
|
pkt103->set_payload(payload103, SrsRtspPacketPayloadTypeRaw);
|
||||||
|
|
||||||
|
// Set the avsync time for the packets to avoid other null pointer issues
|
||||||
|
pkt101->set_avsync_time(1000);
|
||||||
|
pkt102->set_avsync_time(1000);
|
||||||
|
pkt103->set_avsync_time(1000);
|
||||||
|
|
||||||
|
// Store packets in cache (but skip sequence 100 - this is the missing start packet)
|
||||||
|
frame_builder.cache_video_pkts_[frame_builder.cache_index(pkt101->header.get_sequence())].pkt = pkt101;
|
||||||
|
frame_builder.cache_video_pkts_[frame_builder.cache_index(pkt102->header.get_sequence())].pkt = pkt102;
|
||||||
|
frame_builder.cache_video_pkts_[frame_builder.cache_index(pkt103->header.get_sequence())].pkt = pkt103;
|
||||||
|
|
||||||
|
// Before the fix in PR #4451, calling packet_video_rtmp(100, 103) would crash
|
||||||
|
// because it would try to access cache_video_pkts_[cache_index(100)].pkt which is NULL
|
||||||
|
// and then call pkt->get_avsync_time() causing a null pointer dereference
|
||||||
|
|
||||||
|
// The fix ensures we find the first non-null packet (101) instead of assuming
|
||||||
|
// the start packet (100) exists
|
||||||
|
HELPER_EXPECT_SUCCESS(frame_builder.packet_video_rtmp(100, 103));
|
||||||
|
|
||||||
|
// Verify that a frame was successfully processed
|
||||||
|
EXPECT_EQ(1, bridge.frame_count);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user