From ca46185ace69895eb9fb425f8ea57fcd4d25b375 Mon Sep 17 00:00:00 2001 From: john Date: Tue, 20 Jun 2023 13:20:00 +0800 Subject: [PATCH] Fix crash when process rtcp feedback message. v5.0.159, v6.0.52 (#3591) --------- Co-authored-by: johzzy --- .github/workflows/test.yml | 12 ++++- trunk/doc/CHANGELOG.md | 1 + trunk/src/app/srs_app_rtc_conn.cpp | 10 ++-- trunk/src/app/srs_app_rtc_conn.hpp | 4 +- trunk/src/core/srs_core_version5.hpp | 2 +- trunk/src/kernel/srs_kernel_rtc_rtcp.cpp | 37 +++++---------- trunk/src/kernel/srs_kernel_rtc_rtcp.hpp | 58 ++++++++++++------------ 7 files changed, 58 insertions(+), 66 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5ca048ece6..48e66613d8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -25,8 +25,16 @@ jobs: steps: - name: Download Cache for Cygwin run: | + echo "Generate convert.sh" && + echo "for file in \$(find objs -type l); do" > convert.sh && + echo " REAL=\$(readlink -f \$file) &&" >> convert.sh && + echo " echo \"convert \$file to \$REAL\" &&" >> convert.sh && + echo " rm -rf \$file &&" >> convert.sh && + echo " cp -r \$REAL \$file" >> convert.sh && + echo "done" >> convert.sh && + cat convert.sh && docker run --rm -v $(pwd):/srs -w /usr/local/srs-cache/srs/trunk ossrs/srs:cygwin64-cache \ - tar jcf /srs/objs.tar.bz2 objs && + bash -c "bash /srs/convert.sh && tar cf /srs/objs.tar.bz2 objs" && pwd && du -sh * ################################################################################################################## - uses: actions/upload-artifact@v3 @@ -68,7 +76,7 @@ jobs: shell: C:\cygwin64\bin\bash.exe --login '{0}' run: | WORKDIR=$(cygpath -u $SRS_WORKSPACE) && export PATH=/usr/bin:/usr/local/bin && cd ${WORKDIR} && - pwd && rm -rf /usr/local/srs-cache && mkdir -p /usr/local/srs-cache/srs/trunk && + pwd && rm -rf /usr/local/srs-cache && mkdir -p /usr/local/srs-cache/srs/trunk && ls -lh && tar xf objs.tar.bz2 -C /usr/local/srs-cache/srs/trunk/ && du -sh /usr/local/srs-cache/srs/trunk/* && cd ${WORKDIR}/trunk && ./configure --gb28181=on --utest=on && ls -lh && du -sh * && du -sh objs/* && cd ${WORKDIR}/trunk && make utest && ./objs/srs_utest diff --git a/trunk/doc/CHANGELOG.md b/trunk/doc/CHANGELOG.md index e3b80417f5..a3886d178e 100644 --- a/trunk/doc/CHANGELOG.md +++ b/trunk/doc/CHANGELOG.md @@ -8,6 +8,7 @@ The changelog for SRS. ## SRS 5.0 Changelog +* v5.0, 2023-06-20, Merge [#3591](https://github.com/ossrs/srs/pull/3591): Fix crash when process rtcp feedback message. v5.0.159 (#3591) * v5.0, 2023-06-15, Merge [#3581](https://github.com/ossrs/srs/pull/3581): WHIP: Add OBS support, ensuring compatibility with a unique SDP. v5.0.158 (#3581) * v5.0, 2023-06-05, Fix command injection in demonstration api-server for HTTP callback. v5.0.157 * v5.0, 2023-06-05, Merge [#3565](https://github.com/ossrs/srs/pull/3565): DTLS: Use bio callback to get fragment packet. v5.0.156 (#3565) diff --git a/trunk/src/app/srs_app_rtc_conn.cpp b/trunk/src/app/srs_app_rtc_conn.cpp index ca372144ba..db4ad1d0f6 100644 --- a/trunk/src/app/srs_app_rtc_conn.cpp +++ b/trunk/src/app/srs_app_rtc_conn.cpp @@ -785,7 +785,7 @@ srs_error_t SrsRtcPlayStream::on_rtcp(SrsRtcpCommon* rtcp) SrsRtcpNack* nack = dynamic_cast(rtcp); return on_rtcp_nack(nack); } else if(SrsRtcpType_psfb == rtcp->type()) { - SrsRtcpPsfbCommon* psfb = dynamic_cast(rtcp); + SrsRtcpFbCommon* psfb = dynamic_cast(rtcp); return on_rtcp_ps_feedback(psfb); } else if(SrsRtcpType_xr == rtcp->type()) { SrsRtcpXr* xr = dynamic_cast(rtcp); @@ -865,7 +865,7 @@ srs_error_t SrsRtcPlayStream::on_rtcp_nack(SrsRtcpNack* rtcp) return err; } -srs_error_t SrsRtcPlayStream::on_rtcp_ps_feedback(SrsRtcpPsfbCommon* rtcp) +srs_error_t SrsRtcPlayStream::on_rtcp_ps_feedback(SrsRtcpFbCommon* rtcp) { srs_error_t err = srs_success; @@ -2045,7 +2045,7 @@ srs_error_t SrsRtcConnection::dispatch_rtcp(SrsRtcpCommon* rtcp) // For REMB packet. if (SrsRtcpType_psfb == rtcp->type()) { - SrsRtcpPsfbCommon* psfb = dynamic_cast(rtcp); + SrsRtcpFbCommon* psfb = dynamic_cast(rtcp); if (15 == psfb->get_rc()) { return on_rtcp_feedback_remb(psfb); } @@ -2073,7 +2073,7 @@ srs_error_t SrsRtcConnection::dispatch_rtcp(SrsRtcpCommon* rtcp) required_player_ssrc = nack->get_media_ssrc(); } } else if(SrsRtcpType_psfb == rtcp->type()) { - SrsRtcpPsfbCommon* psfb = dynamic_cast(rtcp); + SrsRtcpFbCommon* psfb = dynamic_cast(rtcp); required_player_ssrc = psfb->get_media_ssrc(); } @@ -2122,7 +2122,7 @@ srs_error_t SrsRtcConnection::on_rtcp_feedback_twcc(char* data, int nb_data) return srs_success; } -srs_error_t SrsRtcConnection::on_rtcp_feedback_remb(SrsRtcpPsfbCommon *rtcp) +srs_error_t SrsRtcConnection::on_rtcp_feedback_remb(SrsRtcpFbCommon *rtcp) { //ignore REMB return srs_success; diff --git a/trunk/src/app/srs_app_rtc_conn.hpp b/trunk/src/app/srs_app_rtc_conn.hpp index ce3f407ad5..0ad8573c91 100644 --- a/trunk/src/app/srs_app_rtc_conn.hpp +++ b/trunk/src/app/srs_app_rtc_conn.hpp @@ -269,7 +269,7 @@ class SrsRtcPlayStream : public ISrsCoroutineHandler, public ISrsReloadHandler private: srs_error_t on_rtcp_xr(SrsRtcpXr* rtcp); srs_error_t on_rtcp_nack(SrsRtcpNack* rtcp); - srs_error_t on_rtcp_ps_feedback(SrsRtcpPsfbCommon* rtcp); + srs_error_t on_rtcp_ps_feedback(SrsRtcpFbCommon* rtcp); srs_error_t on_rtcp_rr(SrsRtcpRR* rtcp); uint32_t get_video_publish_ssrc(uint32_t play_ssrc); // Interface ISrsRtcPLIWorkerHandler @@ -513,7 +513,7 @@ class SrsRtcConnection : public ISrsResource, public ISrsDisposingHandler, publi srs_error_t dispatch_rtcp(SrsRtcpCommon* rtcp); public: srs_error_t on_rtcp_feedback_twcc(char* buf, int nb_buf); - srs_error_t on_rtcp_feedback_remb(SrsRtcpPsfbCommon *rtcp); + srs_error_t on_rtcp_feedback_remb(SrsRtcpFbCommon *rtcp); public: srs_error_t on_dtls_handshake_done(); srs_error_t on_dtls_alert(std::string type, std::string desc); diff --git a/trunk/src/core/srs_core_version5.hpp b/trunk/src/core/srs_core_version5.hpp index ac66a9c734..417e4ce4ed 100644 --- a/trunk/src/core/srs_core_version5.hpp +++ b/trunk/src/core/srs_core_version5.hpp @@ -9,6 +9,6 @@ #define VERSION_MAJOR 5 #define VERSION_MINOR 0 -#define VERSION_REVISION 158 +#define VERSION_REVISION 159 #endif diff --git a/trunk/src/kernel/srs_kernel_rtc_rtcp.cpp b/trunk/src/kernel/srs_kernel_rtc_rtcp.cpp index f46b75f0f2..ad19200eed 100644 --- a/trunk/src/kernel/srs_kernel_rtc_rtcp.cpp +++ b/trunk/src/kernel/srs_kernel_rtc_rtcp.cpp @@ -717,10 +717,6 @@ void SrsRtcpTWCC::clear() next_base_sn_ = 0; } -uint32_t SrsRtcpTWCC::get_media_ssrc() const -{ - return media_ssrc_; -} uint16_t SrsRtcpTWCC::get_base_sn() const { return base_sn_; @@ -746,10 +742,6 @@ vector SrsRtcpTWCC::get_recv_deltas() const return pkt_deltas_; } -void SrsRtcpTWCC::set_media_ssrc(uint32_t ssrc) -{ - media_ssrc_ = ssrc; -} void SrsRtcpTWCC::set_base_sn(uint16_t sn) { base_sn_ = sn; @@ -1217,11 +1209,6 @@ SrsRtcpNack::~SrsRtcpNack() { } -uint32_t SrsRtcpNack::get_media_ssrc() const -{ - return media_ssrc_; -} - vector SrsRtcpNack::get_lost_sns() const { vector sn; @@ -1236,11 +1223,6 @@ bool SrsRtcpNack::empty() return lost_sns_.empty(); } -void SrsRtcpNack::set_media_ssrc(uint32_t ssrc) -{ - media_ssrc_ = ssrc; -} - void SrsRtcpNack::add_lost_sn(uint16_t sn) { lost_sns_.insert(sn); @@ -1377,7 +1359,7 @@ srs_error_t SrsRtcpNack::encode(SrsBuffer *buffer) return err; } -SrsRtcpPsfbCommon::SrsRtcpPsfbCommon() +SrsRtcpFbCommon::SrsRtcpFbCommon() { header_.padding = 0; header_.type = SrsRtcpType_psfb; @@ -1386,22 +1368,22 @@ SrsRtcpPsfbCommon::SrsRtcpPsfbCommon() //ssrc_ = sender_ssrc; } -SrsRtcpPsfbCommon::~SrsRtcpPsfbCommon() +SrsRtcpFbCommon::~SrsRtcpFbCommon() { } -uint32_t SrsRtcpPsfbCommon::get_media_ssrc() const +uint32_t SrsRtcpFbCommon::get_media_ssrc() const { return media_ssrc_; } -void SrsRtcpPsfbCommon::set_media_ssrc(uint32_t ssrc) +void SrsRtcpFbCommon::set_media_ssrc(uint32_t ssrc) { media_ssrc_ = ssrc; } -srs_error_t SrsRtcpPsfbCommon::decode(SrsBuffer *buffer) +srs_error_t SrsRtcpFbCommon::decode(SrsBuffer *buffer) { /* @doc: https://tools.ietf.org/html/rfc4585#section-6.1 @@ -1432,12 +1414,12 @@ srs_error_t SrsRtcpPsfbCommon::decode(SrsBuffer *buffer) return err; } -uint64_t SrsRtcpPsfbCommon::nb_bytes() +uint64_t SrsRtcpFbCommon::nb_bytes() { return kRtcpPacketSize; } -srs_error_t SrsRtcpPsfbCommon::encode(SrsBuffer *buffer) +srs_error_t SrsRtcpFbCommon::encode(SrsBuffer *buffer) { return srs_error_new(ERROR_RTC_RTCP, "not support"); } @@ -1762,6 +1744,9 @@ srs_error_t SrsRtcpCompound::decode(SrsBuffer *buffer) } else if (15 == header->rc) { //twcc rtcp = new SrsRtcpTWCC(); + } else { + // common fb + rtcp = new SrsRtcpFbCommon(); } } else if(header->type == SrsRtcpType_psfb) { if(1 == header->rc) { @@ -1775,7 +1760,7 @@ srs_error_t SrsRtcpCompound::decode(SrsBuffer *buffer) rtcp = new SrsRtcpRpsi(); } else { // common psfb - rtcp = new SrsRtcpPsfbCommon(); + rtcp = new SrsRtcpFbCommon(); } } else if(header->type == SrsRtcpType_xr) { rtcp = new SrsRtcpXr(); diff --git a/trunk/src/kernel/srs_kernel_rtc_rtcp.hpp b/trunk/src/kernel/srs_kernel_rtc_rtcp.hpp index 761932c6fc..1c0dc921e7 100644 --- a/trunk/src/kernel/srs_kernel_rtc_rtcp.hpp +++ b/trunk/src/kernel/srs_kernel_rtc_rtcp.hpp @@ -206,6 +206,28 @@ class SrsRtcpRR : public SrsRtcpCommon }; +// @doc: https://tools.ietf.org/html/rfc4585#section-6.1 +// As RFC 4585 says, all FB messages MUST use a common packet format, +// inlucde Transport layer FB message and Payload-specific FB message. +class SrsRtcpFbCommon : public SrsRtcpCommon +{ +protected: + uint32_t media_ssrc_; +public: + SrsRtcpFbCommon(); + virtual ~SrsRtcpFbCommon(); + + uint32_t get_media_ssrc() const; + void set_media_ssrc(uint32_t ssrc); + +// interface ISrsCodec +public: + virtual srs_error_t decode(SrsBuffer *buffer); + virtual uint64_t nb_bytes(); + virtual srs_error_t encode(SrsBuffer *buffer); +}; + + // The Message format of TWCC, @see https://tools.ietf.org/html/draft-holmer-rmcat-transport-wide-cc-extensions-01#section-3.1 // 0 1 2 3 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 @@ -247,10 +269,9 @@ class SrsRtcpRR : public SrsRtcpCommon #define kTwccFbLargeRecvDeltaBytes 2 #define kTwccFbMaxBitElements kTwccFbOneBitElements -class SrsRtcpTWCC : public SrsRtcpCommon +class SrsRtcpTWCC : public SrsRtcpFbCommon { private: - uint32_t media_ssrc_; uint16_t base_sn_; int32_t reference_time_; uint8_t fb_pkt_count_; @@ -286,14 +307,12 @@ class SrsRtcpTWCC : public SrsRtcpCommon SrsRtcpTWCC(uint32_t sender_ssrc = 0); virtual ~SrsRtcpTWCC(); - uint32_t get_media_ssrc() const; uint16_t get_base_sn() const; uint32_t get_reference_time() const; uint8_t get_feedback_count() const; std::vector get_packet_chucks() const; std::vector get_recv_deltas() const; - void set_media_ssrc(uint32_t ssrc); void set_base_sn(uint16_t sn); void set_reference_time(uint32_t time); void set_feedback_count(uint8_t count); @@ -312,7 +331,7 @@ class SrsRtcpTWCC : public SrsRtcpCommon srs_error_t do_encode(SrsBuffer *buffer); }; -class SrsRtcpNack : public SrsRtcpCommon +class SrsRtcpNack : public SrsRtcpFbCommon { private: struct SrsPidBlp { @@ -321,17 +340,14 @@ class SrsRtcpNack : public SrsRtcpCommon bool in_use; }; - uint32_t media_ssrc_; std::set lost_sns_; public: SrsRtcpNack(uint32_t sender_ssrc = 0); virtual ~SrsRtcpNack(); - uint32_t get_media_ssrc() const; std::vector get_lost_sns() const; bool empty(); - void set_media_ssrc(uint32_t ssrc); void add_lost_sn(uint16_t sn); // interface ISrsCodec public: @@ -340,25 +356,7 @@ class SrsRtcpNack : public SrsRtcpCommon virtual srs_error_t encode(SrsBuffer *buffer); }; -class SrsRtcpPsfbCommon : public SrsRtcpCommon -{ -protected: - uint32_t media_ssrc_; -public: - SrsRtcpPsfbCommon(); - virtual ~SrsRtcpPsfbCommon(); - - uint32_t get_media_ssrc() const; - void set_media_ssrc(uint32_t ssrc); - -// interface ISrsCodec -public: - virtual srs_error_t decode(SrsBuffer *buffer); - virtual uint64_t nb_bytes(); - virtual srs_error_t encode(SrsBuffer *buffer); -}; - -class SrsRtcpPli : public SrsRtcpPsfbCommon +class SrsRtcpPli : public SrsRtcpFbCommon { public: SrsRtcpPli(uint32_t sender_ssrc = 0); @@ -371,7 +369,7 @@ class SrsRtcpPli : public SrsRtcpPsfbCommon virtual srs_error_t encode(SrsBuffer *buffer); }; -class SrsRtcpSli : public SrsRtcpPsfbCommon +class SrsRtcpSli : public SrsRtcpFbCommon { private: uint16_t first_; @@ -388,7 +386,7 @@ class SrsRtcpSli : public SrsRtcpPsfbCommon virtual srs_error_t encode(SrsBuffer *buffer); }; -class SrsRtcpRpsi : public SrsRtcpPsfbCommon +class SrsRtcpRpsi : public SrsRtcpFbCommon { private: uint8_t pb_; @@ -407,7 +405,7 @@ class SrsRtcpRpsi : public SrsRtcpPsfbCommon virtual srs_error_t encode(SrsBuffer *buffer); }; -class SrsRtcpXr : public SrsRtcpCommon +class SrsRtcpXr : public SrsRtcpFbCommon { public: SrsRtcpXr (uint32_t ssrc = 0);