Skip to content

Bug: ethos TEXT-frame escape resets the whole RX tsrb and can drop an unread DATA frame #22606

Description

@andrewlihhh

Description

ETHOS multiplexes ethernet DATA and stdio TEXT on the same UART. Incoming
bytes go through ethos_isr(). A completed DATA frame stays in dev->inbuf
until the netdev thread consumes it in _recv().

When a TEXT-frame type escape arrives, the ISR does:

case (ETHOS_FRAME_TYPE_TEXT ^ 0x20):
    dev->frametype = ETHOS_FRAME_TYPE_TEXT;
    /* reset tsrb (used for networking) */
    dev->inbuf.reads = 0;
    dev->inbuf.writes = 0;
    dev->state = IN_FRAME;
    return;

That zeros the whole networking ring, not only the frame currently being
assembled. An already-delimited DATA frame that _recv() has not finished
reading is discarded. _recv() can then hit tsrb_get_one() < 0 and return
-EIO.

The same file already uses tsrb_clear() in _fail_frame() for a broken DATA
frame. tsrb_clear() also disables IRQs around the update. The TEXT-escape
path uses two raw stores instead.

Typical sequence with stdio-over-ethos:

  1. ISR finishes a DATA frame (delimiter, _end_of_frame(),
    netdev_trigger_event_isr()). The bytes are still in inbuf.
  2. The netdev thread is in _recv(), looping tsrb_get_one().
  3. The next UART frame is TEXT (ESC then ETHOS_FRAME_TYPE_TEXT ^ 0x20).
  4. ISR sets reads = writes = 0.
  5. _recv() sees an empty ring mid-packet and returns -EIO / a truncated
    frame.

Suggested direction: do not wipe the whole inbuf. Drop only the bytes of the
frame currently being assembled. If a full clear is really intended, use
tsrb_clear().

File: drivers/ethos/ethos.c
Line numbers on current master, in drivers/ethos/ethos.c:

  • TEXT frame type in ethos_isr(): lines 177 to 183 (zeros inbuf.reads and inbuf.writes)
  • _recv(): lines 354 to 371 (returns -EIO if the ring is empty)
  • _fail_frame(): line 83 (uses tsrb_clear())

Steps to reproduce the issue

  1. Receive a complete DATA frame into dev->inbuf and start _recv().
  2. After _recv() has consumed at least one byte but not the delimiter, feed
    ESC followed by (ETHOS_FRAME_TYPE_TEXT ^ 0x20) on the UART.
  3. The remainder of the DATA frame is gone from inbuf.

Any board that uses ethos with both DATA and TEXT frames can hit this (stdio
over ethos).

Expected results

A TEXT frame should not destroy an unread, already-delimited DATA frame sitting
in inbuf.

Actual results

The whole ring is reset. The in-progress _recv() loses the packet.

Versions

  • RIOT: current master (drivers/ethos/ethos.c)
  • OS: Linux
  • This is a source-level race between ethos_isr() and _recv().

Declaration of AI-Tools / LLMs usage:

AI-Tools / LLMs that were used are:

  • xAI Grok for drafting this issue text, with user review

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions