Arithmetic overflow in fragment size calculations (#5464)

* Tests arithmetic overflow in fragment size calculations Signed-off-by: Eugenio Collado <[email protected]> * Refs #21814. Fix code in BaseWriter.cpp. Signed-off-by: Miguel Company <[email protected]> * Fix corner case overhead==max_data_size Signed-off-by: Eugenio Collado <[email protected]> * Refs #21814. Fix code in WriterHistory.cpp. Signed-off-by: Miguel Company <[email protected]> * Fix corner case overhead==final_high_mark_for_frag Signed-off-by: Eugenio Collado <[email protected]> * Uncrustify Signed-off-by: Eugenio Collado <[email protected]> * Fix log error message Signed-off-by: Eugenio Collado <[email protected]> * Fix test fragments not been dropped Signed-off-by: Eugenio Collado <[email protected]> * Fix corner case RTPSParticipantImpl max_data_size < overhead Signed-off-by: Eugenio Collado <[email protected]> * Test refactor for windows compilation Signed-off-by: Eugenio Collado <[email protected]> * Fix blackbox test Signed-off-by: Eugenio Collado <[email protected]> * Applied review suggestions Signed-off-by: EugenioCollado <[email protected]> --------- Signed-off-by: Eugenio Collado <[email protected]> Signed-off-by: Miguel Company <[email protected]> Signed-off-by: EugenioCollado <[email protected]> Co-authored-by: Miguel Company <[email protected]> (cherry picked from commit bfc5a53) # Conflicts: # src/cpp/rtps/writer/BaseWriter.cpp # test/blackbox/common/DDSBlackboxTestsListeners.cpp # test/unittest/rtps/history/CMakeLists.txt
eProsima · Dec 18, 2024 · 0f341bc · 0f341bc
1 parent 28552ce
commit 0f341bc
Show file tree

Hide file tree

Showing 6 changed files with 591 additions and 10 deletions.
diff --git a/src/cpp/rtps/history/WriterHistory.cpp b/src/cpp/rtps/history/WriterHistory.cpp
@@ -320,9 +320,16 @@ void WriterHistory::set_fragments(
     // If inlineqos for related_sample_identity is required, then remove its size from the final fragment size.
     if (0 < inline_qos_size)
     {
-        final_high_mark_for_frag -= (
-            fastdds::dds::ParameterSerializer<Parameter_t>::PARAMETER_SENTINEL_SIZE +
-            inline_qos_size);
+        uint32_t overhead = fastdds::dds::ParameterSerializer<Parameter_t>::PARAMETER_SENTINEL_SIZE + inline_qos_size;
+        constexpr uint32_t min_fragment_size = 4;
+        if (final_high_mark_for_frag < (overhead + min_fragment_size))
+        {
+            final_high_mark_for_frag = min_fragment_size;
+        }
+        else
+        {
+            final_high_mark_for_frag -= overhead;
+        }
     }
 
     // If it is big data, fragment it.

diff --git a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp
@@ -2390,20 +2390,22 @@ uint32_t RTPSParticipantImpl::getMaxDataSize()
 uint32_t RTPSParticipantImpl::calculateMaxDataSize(
         uint32_t length)
 {
-    uint32_t maxDataSize = length;
-
+    // RTPS header
+    uint32_t overhead = RTPSMESSAGE_HEADER_SIZE;
 #if HAVE_SECURITY
     // If there is rtps messsage protection, reduce max size for messages,
     // because extra data is added on encryption.
     if (security_attributes_.is_rtps_protected)
     {
-        maxDataSize -= m_security_manager.calculate_extra_size_for_rtps_message();
+        overhead += m_security_manager.calculate_extra_size_for_rtps_message();
     }
 #endif // if HAVE_SECURITY
 
-    // RTPS header
-    maxDataSize -= RTPSMESSAGE_HEADER_SIZE;
-    return maxDataSize;
+    if (length <= overhead)
+    {
+        return 0;
+    }
+    return length - overhead;
 }
 
 bool RTPSParticipantImpl::networkFactoryHasRegisteredTransports() const