Skip to content

Fix volatile reader desync after initial positioning GAP#6456

Open
cferreiragonz wants to merge 1 commit into
masterfrom
backport/fix_gap
Open

Fix volatile reader desync after initial positioning GAP#6456
cferreiragonz wants to merge 1 commit into
masterfrom
backport/fix_gap

Conversation

@cferreiragonz

Copy link
Copy Markdown
Contributor

Description

This PR is an alternative to fix proposed for CVE-2025-64438 that limited the maximum number of GAPs that a reader could process. This fix had a problem with volatile late-joiners readers that caused a de-synchronization between reader and writer, resulting in the reader not processing any message. The key is that the writer assumes the reader's state after sending the first initial positioning GAP in StatefulWriter::matched_reader_add_edp. Hence, later ACKs requesting old data are simply ignored and the reader never syncs with the writer, preventing real communication even though DATA is correctly being received at the transport layer.

The issue comes, thus, from two different conditions:

  • StatefulReader not processing all GAPs sent by the writer (similar behavior will happen if the GAP message is lost or sent before the reader match the writer on its side - Discovery race condition)
  • StatefulWriter assuming the reader's state after sending a positioning GAP

The new fix forces sending a HB before every GAP and limiting the GAP processing to the value specified in the heartbeat. It also allows the Writer to send GAP messages if a later ACK is received requesting old samples.

@Mergifyio backport 3.2.x 2.14.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
  • 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.

* Refs #24631: Regression Test

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>

* Refs #24631: Limit GAP processing to HB announced value

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>

* Refs #24631: Send a HB before every GAP

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>

* Refs #24631: Update tests

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>

* Refs #24631: Fix writer's assumption of reader state after GAP sent

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>

* Refs #24631: Uncrustify

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>

* Refs #24631: Update versions.md

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>

* Refs #24631: Skip GAPs for changes removed from history

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>

* Refs #24631: Fix Datasharing GAP reception

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>

* Refs #24631: Handle sample processed as lost instead of GAP'ed

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>

* Refs #24631: Send HB in process_acknack iff GAP is required

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>

* Refs #24631: Coverage test for unknown sequence number comparison

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>

---------

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
@cferreiragonz cferreiragonz added this to the v3.7.0 milestone Jun 26, 2026
@github-actions github-actions Bot added the ci-pending PR which CI is running label Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-pending PR which CI is running

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant