[core] Fix CRcvBuffer last position in getTimespan_ms (#2579).

Haivision · Dec 14, 2022 · de9fc45 · de9fc45
1 parent 60d1237
commit de9fc45
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 27 deletions.
diff --git a/srtcore/buffer_rcv.cpp b/srtcore/buffer_rcv.cpp
@@ -591,19 +591,23 @@ int CRcvBuffer::getTimespan_ms() const
     if (m_iMaxPosInc == 0)
         return 0;
 
-    const int lastpos = incPos(m_iStartPos, m_iMaxPosInc - 1);
-    // Should not happen if TSBPD is enabled (reading out of order is not allowed).
-    SRT_ASSERT(m_entries[lastpos].pUnit != NULL);
+    int lastpos = incPos(m_iStartPos, m_iMaxPosInc - 1);
+    // Normally the last position should always be non empty
+    // if TSBPD is enabled (reading out of order is not allowed).
+    // However if decryption of the last packet fails, it may be dropped
+    // from the buffer (AES-GCM), and the position will be empty.
+    SRT_ASSERT(m_entries[lastpos].pUnit != NULL || m_entries[lastpos].status == EntryState_Drop);
+    while (m_entries[lastpos].pUnit == NULL && lastpos != m_iStartPos)
+    {
+        lastpos = decPos(lastpos);
+    }
+
     if (m_entries[lastpos].pUnit == NULL)
         return 0;
 
     int startpos = m_iStartPos;
-
-    while (m_entries[startpos].pUnit == NULL)
+    while (m_entries[startpos].pUnit == NULL && startpos != lastpos)
     {
-        if (startpos == lastpos)
-            break;
-
         startpos = incPos(startpos);
     }
 

diff --git a/srtcore/core.cpp b/srtcore/core.cpp
@@ -9820,11 +9820,11 @@ int srt::CUDT::handleSocketPacketReception(const vector<CUnit*>& incoming, bool&
             excessive = false;
             if (u->m_Packet.getMsgCryptoFlags() != EK_NOENC)
             {
-                    // TODO: reset and restore the timestamp if TSBPD is disabled.
-                    // Reset retransmission flag (must be excluded from GCM auth tag).
-                    u->m_Packet.setRexmitFlag(false);
-                    const EncryptionStatus rc = m_pCryptoControl ? m_pCryptoControl->decrypt((u->m_Packet)) : ENCS_NOTSUP;
-                    u->m_Packet.setRexmitFlag(retransmitted); // Recover the flag.
+                // TODO: reset and restore the timestamp if TSBPD is disabled.
+                // Reset retransmission flag (must be excluded from GCM auth tag).
+                u->m_Packet.setRexmitFlag(false);
+                const EncryptionStatus rc = m_pCryptoControl ? m_pCryptoControl->decrypt((u->m_Packet)) : ENCS_NOTSUP;
+                u->m_Packet.setRexmitFlag(retransmitted); // Recover the flag.
 
                 if (rc != ENCS_CLEAR)
                 {
@@ -9833,20 +9833,20 @@ int srt::CUDT::handleSocketPacketReception(const vector<CUnit*>& incoming, bool&
                     adding_successful = false;
                     IF_HEAVY_LOGGING(exc_type = "UNDECRYPTED");
 
-                        if (m_config.iCryptoMode == CSrtConfig::CIPHER_MODE_AES_GCM)
-                        {
-                            // Drop a packet from the receiver buffer.
-                            // Dropping depends on the configuration mode. If message mode is enabled, we have to drop the whole message.
-                            // Otherwise just drop the exact packet.
-                            if (m_config.bMessageAPI)
-                                m_pRcvBuffer->dropMessage(SRT_SEQNO_NONE, SRT_SEQNO_NONE, u->m_Packet.getMsgSeq(m_bPeerRexmitFlag));
-                            else
-                                m_pRcvBuffer->dropMessage(u->m_Packet.getSeqNo(), u->m_Packet.getSeqNo(), SRT_MSGNO_NONE);
-
-                            LOGC(qrlog.Error, log << CONID() << "AEAD decryption failed, breaking the connection.");
-                            m_bBroken = true;
-                            m_iBrokenCounter = 0;
-                        }
+                    if (m_config.iCryptoMode == CSrtConfig::CIPHER_MODE_AES_GCM)
+                    {
+                        // Drop a packet from the receiver buffer.
+                        // Dropping depends on the configuration mode. If message mode is enabled, we have to drop the whole message.
+                        // Otherwise just drop the exact packet.
+                        if (m_config.bMessageAPI)
+                            m_pRcvBuffer->dropMessage(SRT_SEQNO_NONE, SRT_SEQNO_NONE, u->m_Packet.getMsgSeq(m_bPeerRexmitFlag));
+                        else
+                            m_pRcvBuffer->dropMessage(u->m_Packet.getSeqNo(), u->m_Packet.getSeqNo(), SRT_MSGNO_NONE);
+
+                        LOGC(qrlog.Error, log << CONID() << "AEAD decryption failed, breaking the connection.");
+                        m_bBroken = true;
+                        m_iBrokenCounter = 0;
+                    }
 
                     ScopedLock lg(m_StatsLock);
                     m_stats.rcvr.undecrypted.count(stats::BytesPackets(rpkt.getLength(), 1));