Fix crash when process rtcp feedback message. v5.0.159, v6.0.52 (#3591)

--------- Co-authored-by: johzzy <hellojinqiang@gmail.com>
ossrs · Jun 20, 2023 · ca46185 · ca46185
1 parent 5759025
commit ca46185
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 66 deletions.
diff --git 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
@@ -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
@@ -785,7 +785,7 @@ srs_error_t SrsRtcPlayStream::on_rtcp(SrsRtcpCommon* rtcp)
         SrsRtcpNack* nack = dynamic_cast<SrsRtcpNack*>(rtcp);
         return on_rtcp_nack(nack);
     } else if(SrsRtcpType_psfb == rtcp->type()) {
-        SrsRtcpPsfbCommon* psfb = dynamic_cast<SrsRtcpPsfbCommon*>(rtcp);
+        SrsRtcpFbCommon* psfb = dynamic_cast<SrsRtcpFbCommon*>(rtcp);
         return on_rtcp_ps_feedback(psfb);
     } else if(SrsRtcpType_xr == rtcp->type()) {
         SrsRtcpXr* xr = dynamic_cast<SrsRtcpXr*>(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<SrsRtcpPsfbCommon*>(rtcp);
+        SrsRtcpFbCommon* psfb = dynamic_cast<SrsRtcpFbCommon*>(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<SrsRtcpPsfbCommon*>(rtcp);
+        SrsRtcpFbCommon* psfb = dynamic_cast<SrsRtcpFbCommon*>(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
@@ -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
@@ -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
@@ -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<uint16_t> 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<uint16_t> SrsRtcpNack::get_lost_sns() const
 {
     vector<uint16_t> 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
@@ -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<uint16_t> get_packet_chucks() const;
     std::vector<uint16_t> 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<uint16_t, SrsSeqCompareLess> lost_sns_;
 public:
     SrsRtcpNack(uint32_t sender_ssrc = 0);
     virtual ~SrsRtcpNack();
 
-    uint32_t get_media_ssrc() const;
     std::vector<uint16_t> 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);