[22438] Arithmetic overflow in fragment size calculations #5464

EugenioCollado · 2024-12-05T08:21:58Z

Description

This PR fixes an overflow in the WriterHistory and BaseWriter that occurred when the maximum message size was insufficient to accommodate the message's inherent headers.

@Mergifyio backport 3.1.x 3.0.x 2.14.x 2.10.x

Contributor Checklist

Commit messages follow the project guidelines.
The code follows the style guidelines of this project.
Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
N/A Any new/modified methods have been properly documented using Doxygen.
N/A Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
Changes are backport compatible: they do NOT break ABI nor change library core behavior.
Changes are API compatible.
N/A New feature has been added to the versions.md file (if applicable).
N/A New feature has been documented/Current behavior is correctly described in the documentation.
Applicable backports have been included in the description.

Reviewer Checklist

The PR has a milestone assigned.
The title and description correctly express the PR's purpose.
Check contributor checklist is correct.
If this is a critical bug fix, backports to the critical-only supported branches have been requested.
Check CI results: changes do not issue any warning.
Check CI results: failing tests are unrelated with the changes.

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

test/unittest/rtps/history/CMakeLists.txt

Signed-off-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com>

MiguelCompany

LGTM with green CI

EugenioCollado · 2024-12-18T08:12:59Z

@Mergifyio backport 3.1.x 3.0.x 2.14.x 2.10.x

mergify · 2024-12-18T08:13:27Z

backport 3.1.x 3.0.x 2.14.x 2.10.x

✅ Backports have been created

#5511 [22438] Arithmetic overflow in fragment size calculations (backport #5464) has been created for branch 3.1.x
#5512 [22438] Arithmetic overflow in fragment size calculations (backport #5464) has been created for branch 3.0.x
#5513 [22438] Arithmetic overflow in fragment size calculations (backport #5464) has been created for branch 2.14.x but encountered conflicts
#5514 [22438] Arithmetic overflow in fragment size calculations (backport #5464) has been created for branch 2.10.x but encountered conflicts

* Tests arithmetic overflow in fragment size calculations Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Refs #21814. Fix code in BaseWriter.cpp. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Fix corner case overhead==max_data_size Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Refs #21814. Fix code in WriterHistory.cpp. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Fix corner case overhead==final_high_mark_for_frag Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Uncrustify Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix log error message Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix test fragments not been dropped Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix corner case RTPSParticipantImpl max_data_size < overhead Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Test refactor for windows compilation Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix blackbox test Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Applied review suggestions Signed-off-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com> --------- Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> Signed-off-by: Miguel Company <miguelcompany@eprosima.com> Signed-off-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com> Co-authored-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit bfc5a53) # Conflicts: # src/cpp/rtps/writer/BaseWriter.cpp # test/blackbox/common/DDSBlackboxTestsListeners.cpp

* Tests arithmetic overflow in fragment size calculations Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Refs #21814. Fix code in BaseWriter.cpp. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Fix corner case overhead==max_data_size Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Refs #21814. Fix code in WriterHistory.cpp. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Fix corner case overhead==final_high_mark_for_frag Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Uncrustify Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix log error message Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix test fragments not been dropped Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix corner case RTPSParticipantImpl max_data_size < overhead Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Test refactor for windows compilation Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix blackbox test Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Applied review suggestions Signed-off-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com> --------- Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> Signed-off-by: Miguel Company <miguelcompany@eprosima.com> Signed-off-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com> Co-authored-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit bfc5a53)

* Tests arithmetic overflow in fragment size calculations Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Refs #21814. Fix code in BaseWriter.cpp. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Fix corner case overhead==max_data_size Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Refs #21814. Fix code in WriterHistory.cpp. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Fix corner case overhead==final_high_mark_for_frag Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Uncrustify Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix log error message Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix test fragments not been dropped Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix corner case RTPSParticipantImpl max_data_size < overhead Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Test refactor for windows compilation Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix blackbox test Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Applied review suggestions Signed-off-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com> --------- Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> Signed-off-by: Miguel Company <miguelcompany@eprosima.com> Signed-off-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com> Co-authored-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit bfc5a53) # Conflicts: # src/cpp/rtps/writer/BaseWriter.cpp # test/blackbox/common/DDSBlackboxTestsListeners.cpp # test/unittest/rtps/history/CMakeLists.txt

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

* Tests arithmetic overflow in fragment size calculations Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Refs #21814. Fix code in BaseWriter.cpp. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Fix corner case overhead==max_data_size Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Refs #21814. Fix code in WriterHistory.cpp. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Fix corner case overhead==final_high_mark_for_frag Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Uncrustify Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix log error message Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix test fragments not been dropped Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix corner case RTPSParticipantImpl max_data_size < overhead Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Test refactor for windows compilation Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix blackbox test Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Applied review suggestions Signed-off-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com> --------- Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> Signed-off-by: Miguel Company <miguelcompany@eprosima.com> Signed-off-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com> Co-authored-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit bfc5a53)

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> Co-authored-by: Eugenio Collado <eugeniocollado@eprosima.com>

* Tests arithmetic overflow in fragment size calculations Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Refs #21814. Fix code in BaseWriter.cpp. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Fix corner case overhead==max_data_size Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Refs #21814. Fix code in WriterHistory.cpp. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Fix corner case overhead==final_high_mark_for_frag Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Uncrustify Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix log error message Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix test fragments not been dropped Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix corner case RTPSParticipantImpl max_data_size < overhead Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Test refactor for windows compilation Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix blackbox test Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Applied review suggestions Signed-off-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com> --------- Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> Signed-off-by: Miguel Company <miguelcompany@eprosima.com> Signed-off-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com> Co-authored-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit bfc5a53) Co-authored-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com>

* Tests arithmetic overflow in fragment size calculations Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Refs #21814. Fix code in BaseWriter.cpp. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Fix corner case overhead==max_data_size Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Refs #21814. Fix code in WriterHistory.cpp. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Fix corner case overhead==final_high_mark_for_frag Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Uncrustify Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix log error message Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix test fragments not been dropped Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix corner case RTPSParticipantImpl max_data_size < overhead Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Test refactor for windows compilation Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Fix blackbox test Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> * Applied review suggestions Signed-off-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com> --------- Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> Signed-off-by: Miguel Company <miguelcompany@eprosima.com> Signed-off-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com> Co-authored-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit bfc5a53) Co-authored-by: Eugenio Collado <eugeniocollado@eprosima.com>

EugenioCollado requested a review from richiprosima December 5, 2024 08:22

github-actions bot added the ci-pending PR which CI is running label Dec 5, 2024

EugenioCollado force-pushed the fix/21814 branch from 91e53d8 to 2042237 Compare December 5, 2024 08:55

EugenioCollado requested review from richiprosima and removed request for richiprosima December 5, 2024 08:57

EugenioCollado added this to the v3.2.0 milestone Dec 5, 2024

EugenioCollado marked this pull request as ready for review December 5, 2024 09:34

EugenioCollado changed the title ~~[21814] Arithmetic overflow in fragment size calculations~~ [22438] Arithmetic overflow in fragment size calculations Dec 5, 2024

EugenioCollado requested review from MiguelCompany and removed request for richiprosima December 5, 2024 09:45

EugenioCollado force-pushed the fix/21814 branch 2 times, most recently from 7234fc3 to 549428a Compare December 9, 2024 14:49

MiguelCompany requested review from MiguelCompany and removed request for MiguelCompany December 9, 2024 15:18

EugenioCollado mentioned this pull request Dec 9, 2024

[21913] DomainParticipant creation fails if SHM MaxMessageSize is lower than minimum #5473

Open

12 tasks

EugenioCollado force-pushed the fix/21814 branch 2 times, most recently from 0422b3b to 07a4d8c Compare December 13, 2024 10:38

EugenioCollado and others added 11 commits December 16, 2024 08:07

Tests arithmetic overflow in fragment size calculations

f85b26b

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

Refs #21814. Fix code in BaseWriter.cpp.

2d4823d

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

Fix corner case overhead==max_data_size

95a17e5

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

Refs #21814. Fix code in WriterHistory.cpp.

805291c

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

Fix corner case overhead==final_high_mark_for_frag

0823f10

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

Uncrustify

a180b98

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

Fix log error message

da7b1c6

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

Fix test fragments not been dropped

c9a4272

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

Fix corner case RTPSParticipantImpl max_data_size < overhead

fe4dca7

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

Test refactor for windows compilation

99f9c28

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

Fix blackbox test

a92ba0f

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

EugenioCollado force-pushed the fix/21814 branch from 63e6bdc to a92ba0f Compare December 16, 2024 07:08

EugenioCollado requested a review from richiprosima December 16, 2024 07:08

EugenioCollado requested review from richiprosima and removed request for richiprosima December 16, 2024 08:15

MiguelCompany reviewed Dec 17, 2024

View reviewed changes

test/unittest/rtps/history/CMakeLists.txt Outdated Show resolved Hide resolved

Applied review suggestions

0467bbb

Signed-off-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com>

MiguelCompany self-requested a review December 17, 2024 15:01

MiguelCompany approved these changes Dec 17, 2024

View reviewed changes

MiguelCompany added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed ci-pending PR which CI is running labels Dec 18, 2024

MiguelCompany merged commit bfc5a53 into master Dec 18, 2024
16 of 17 checks passed

MiguelCompany deleted the fix/21814 branch December 18, 2024 08:30

mergify bot mentioned this pull request Dec 18, 2024

[22438] Arithmetic overflow in fragment size calculations (backport #5464) #5514

Merged

12 tasks

EugenioCollado added a commit that referenced this pull request Dec 19, 2024

Arithmetic overflow in fragment size calculations (backport #5464

0d42153

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

EugenioCollado added a commit that referenced this pull request Dec 20, 2024

Arithmetic overflow in fragment size calculations (backport #5464

43fbb20

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>

EugenioCollado added a commit that referenced this pull request Dec 27, 2024

Arithmetic overflow in fragment size calculations (#5464) (#5513)

29ff526

Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com> Co-authored-by: Eugenio Collado <eugeniocollado@eprosima.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[22438] Arithmetic overflow in fragment size calculations #5464

[22438] Arithmetic overflow in fragment size calculations #5464

EugenioCollado commented Dec 5, 2024 •

edited by MiguelCompany

Loading

MiguelCompany left a comment

EugenioCollado commented Dec 18, 2024 •

edited

Loading

mergify bot commented Dec 18, 2024 •

edited

Loading

[22438] Arithmetic overflow in fragment size calculations #5464

[22438] Arithmetic overflow in fragment size calculations #5464

Conversation

EugenioCollado commented Dec 5, 2024 • edited by MiguelCompany Loading

Description

Contributor Checklist

Reviewer Checklist

MiguelCompany left a comment

Choose a reason for hiding this comment

EugenioCollado commented Dec 18, 2024 • edited Loading

mergify bot commented Dec 18, 2024 • edited Loading

✅ Backports have been created

EugenioCollado commented Dec 5, 2024 •

edited by MiguelCompany

Loading

EugenioCollado commented Dec 18, 2024 •

edited

Loading

mergify bot commented Dec 18, 2024 •

edited

Loading