Skip to content

fix(rmw): check implementation_identifier before casting guard condition data#8

Merged
SandraK82 merged 1 commit into
zero-objects:mainfrom
Rico1502:fix/guard-condition-data-check
Jul 8, 2026
Merged

fix(rmw): check implementation_identifier before casting guard condition data#8
SandraK82 merged 1 commit into
zero-objects:mainfrom
Rico1502:fix/guard-condition-data-check

Conversation

@Rico1502

@Rico1502 Rico1502 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Bei der Verwendung von zerodds als RMW-Implementierung mit rclrs (der reinen Rust-ROS2-Client-Bibliothek) stürzt der Knoten sofort mit einem Segfault in rmw_wait ab.

rclrs erstellt intern über rcl (rcl_wait_set_init) Guard-Bedingungen für die Verwaltung seiner Executor- und Wait-Sets. Diese Guard-Bedingungen werden nicht über rmw_create_guard_condition erstellt, was bedeutet, dass ihr data-Feld kein unsigned int * ist, wie es der C-Wrapper erwartet - es enthält stattdessen das, was rcl dort abgelegt hat.

Wenn rmw_wait die Guard-Bedingungen durchläuft und gc->data in unsigned int * casted und dereferenziert, liest es nicht definierten Speicher und es kommt zu einem Segfault.

Grundursache

rmw_create_guard_condition setzt gc->data korrekt auf ein im Heap zugewiesenes unsigned int *-Flag. Allerdings erstellt rcl intern zusätzliche Guard-Bedingungen (für das Aufwachen des Executors und die Verwaltung der Wait-Sets), die rmw_create_guard_condition vollständig umgehen. Diese Guard-Bedingungen landen im an rmw_wait übergebenen Wait-Set-Array, aber ihr data-Feld ist kein unsigned int *.

Spec-Bezug

N/A - interner Bugfix im RMW-C-Wrapper, kein OMG-Spec-Verhalten betroffen.

Test-Coverage

  • Neue Tests für neuen Code geschrieben
  • Bestehende Tests berücksichtigen die Änderung
  • cargo test --workspace lokal grün
  • cargo clippy --workspace --all-targets -- -D warnings lokal grün
  • cargo fmt --all -- --check lokal grün

Breaking Change?

Nein

Checklist

  • Conventional-Commit-Title (fix(rmw): ...)
  • Crate-CHANGELOG.md aktualisiert (falls Public-API betroffen)
  • Spec-Coverage-Doc aktualisiert (falls Spec-Verhalten betroffen)

Comments

Lokale Umgebung

Überprüft durch Ausführen eines reinen Rust-ROS2-Knotens mit rclrs 0.7.0 auf ROS Humble. Vor der Korrektur stürzte der Knoten beim Start sofort ab (Segfault). Nach der Korrektur läuft der Knoten korrekt mit Subscribern und Publishern.

Getestet mit: zerodds, aus dem Quellcode kompiliert (1.0.0-rc.4) über Nix/naersk, rclrs 0.7.0, ROS Humble in einer nix-shell für x86_64.

Test-Coverage

Ich nutze die crate in einer Nix Umgebung, daher habe ich mir nur die minimal notwendigsten Abhängigkeiten für rmw-zerodds-shim vorbereitet, so dass die tests nicht durchlaufen können. Weiterhin besteht die Änderung hier nur aus einer C Datei. Vermutlich irrelevant für die Tests

AI

Viele Ideen hier sind in Zusammenarbeit mit Claude entstanden, da ich insbesondere noch meine ROS2 Nix Umgebung anpassen musste.

@SandraK82

Copy link
Copy Markdown
Contributor

Danke dir, Rico — sauber analysiert und genau richtig lokalisiert. Der blinde Cast von gc->data bei fremden (nicht über rmw_create_guard_condition erzeugten) Guard-Conditions ist echt, und die implementation_identifier-Prüfung ist der korrekte Fix. Merge ich.

Ein Hinweis für alle Mitleser: derselbe ungeschützte Cast steht ein zweites Mal in der Finalize-Schleife von rmw_wait (weiter unten, dort wird *f sogar geschrieben) — den ziehe ich direkt als Folge-Commit nach, damit die Lücke vollständig zu ist. Nochmal danke fürs Melden und den Repro auf echter Hardware!

@SandraK82 SandraK82 merged commit 7558f04 into zero-objects:main Jul 8, 2026
SandraK82 added a commit that referenced this pull request Jul 8, 2026
Follow-up to #8: the finalize loop of rmw_wait had the same unguarded
cast of gc->data as the readiness loop. Because it also writes `*f = 0`,
it would corrupt rcl's foreign memory (guard conditions rcl created
without rmw_create_guard_condition), not just read it. Apply the same
implementation_identifier check here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants