Skip to content

fix: recompute SoE payload pointer in ecx_SoEread - #985

Open
sankalpsthakur wants to merge 1 commit into
OpenEtherCATsociety:masterfrom
sankalpsthakur:fix/981-soeread-stale-mp
Open

sankalpsthakur wants to merge 1 commit into
OpenEtherCATsociety:masterfrom
sankalpsthakur:fix/981-soeread-stale-mp

Conversation

@sankalpsthakur

@sankalpsthakur sankalpsthakur commented Sep 5, 2026

Copy link
Copy Markdown

Fixes #981.

ecx_SoEread used a NULL-derived payload pointer when reading a valid SoE response. Recompute mp from the received mailbox before copying the payload.

Validation used Clang with AddressSanitizer and a local mailbox mock. A valid one-byte response returned 0x41; a zero-length malformed response reported a packet error and left the destination unchanged. The mock is not included in this PR. The native macOS CMake build was blocked by the missing cmake/Darwin.cmake, so the source check used generated options and Linux OSAL/OSHW headers.

Real servo/NIC behavior, fragmentation and mailbox timing remain untested.

AI coding tools assisted with the change and this description.

ecx_SoEread derived mp from MbxIn while it was still NULL, then memcpy'd
from that stale pointer after ecx_mbxreceive updated MbxIn. A slave
ReadRes therefore read address 0x0a and crashed the master.

Recompute mp from the received mailbox on an accepted SoE ReadRes, and
reject a negative framedatasize before memcpy.

Fixes OpenEtherCATsociety#981
@CLAassistant

CLAassistant commented Sep 5, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@Eksan0325 Eksan0325 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SOEM has always been missing servo communication unit tests. have you set up any test conditions for changes in the SoE?

@sankalpsthakur

sankalpsthakur commented Sep 8, 2026

Copy link
Copy Markdown
Author

At 5487d13, the local mailbox mock passes with Clang, -Wall -Wextra -Werror and AddressSanitizer:

  • After an empty initial flush, a matching single-fragment SoE ReadRes returns 0x41 with the expected size and no packet error.
  • A zero-length mailbox response fails with a packet error and leaves the destination byte unchanged.

The harness stubs mailbox calls and is not in this PR. Fragmentation, timing, servo and NIC behavior remain untested. Would an in-tree mock be useful, and which runner/location fits the project?

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.

Invalid payload pointer in SOEM ecx_SoEread() can cause master-process denial of service

3 participants