Skip to content

Commit 0f341bc

Browse files
EugenioColladomergify[bot]
authored andcommitted
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
1 parent 28552ce commit 0f341bc

File tree

6 files changed

+591
-10
lines changed

6 files changed

+591
-10
lines changed

src/cpp/rtps/history/WriterHistory.cpp

+10-3
Original file line numberDiff line numberDiff line change
@@ -320,9 +320,16 @@ void WriterHistory::set_fragments(
320320
// If inlineqos for related_sample_identity is required, then remove its size from the final fragment size.
321321
if (0 < inline_qos_size)
322322
{
323-
final_high_mark_for_frag -= (
324-
fastdds::dds::ParameterSerializer<Parameter_t>::PARAMETER_SENTINEL_SIZE +
325-
inline_qos_size);
323+
uint32_t overhead = fastdds::dds::ParameterSerializer<Parameter_t>::PARAMETER_SENTINEL_SIZE + inline_qos_size;
324+
constexpr uint32_t min_fragment_size = 4;
325+
if (final_high_mark_for_frag < (overhead + min_fragment_size))
326+
{
327+
final_high_mark_for_frag = min_fragment_size;
328+
}
329+
else
330+
{
331+
final_high_mark_for_frag -= overhead;
332+
}
326333
}
327334

328335
// If it is big data, fragment it.

src/cpp/rtps/participant/RTPSParticipantImpl.cpp

+8-6
Original file line numberDiff line numberDiff line change
@@ -2390,20 +2390,22 @@ uint32_t RTPSParticipantImpl::getMaxDataSize()
23902390
uint32_t RTPSParticipantImpl::calculateMaxDataSize(
23912391
uint32_t length)
23922392
{
2393-
uint32_t maxDataSize = length;
2394-
2393+
// RTPS header
2394+
uint32_t overhead = RTPSMESSAGE_HEADER_SIZE;
23952395
#if HAVE_SECURITY
23962396
// If there is rtps messsage protection, reduce max size for messages,
23972397
// because extra data is added on encryption.
23982398
if (security_attributes_.is_rtps_protected)
23992399
{
2400-
maxDataSize -= m_security_manager.calculate_extra_size_for_rtps_message();
2400+
overhead += m_security_manager.calculate_extra_size_for_rtps_message();
24012401
}
24022402
#endif // if HAVE_SECURITY
24032403

2404-
// RTPS header
2405-
maxDataSize -= RTPSMESSAGE_HEADER_SIZE;
2406-
return maxDataSize;
2404+
if (length <= overhead)
2405+
{
2406+
return 0;
2407+
}
2408+
return length - overhead;
24072409
}
24082410

24092411
bool RTPSParticipantImpl::networkFactoryHasRegisteredTransports() const

0 commit comments

Comments
 (0)