oxidecomputer / oxidecomputer/propolis

server serial task's send-to-listeners future is not cancel-safe

Open
#650 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

server
Dominant language
Rust
Stars
270
Forks
42
Avg merge
4d 5h
Merged PRs (30d)
6

Description

Discovered this during some ad hoc testing with multiple connections to a single guest's serial console, where I observed some data being sent a second time to a listener that had already received it.

If the serial console task has read some data from the guest that needs to be written to listeners, it generates a future to do that writing and then includes it in the main select! that determines the task's next action (lines 116-125 below where cur_output is Some): https://github.com/oxidecomputer/propolis/blob/1b34075ee5dd4235dc7eb46061e1e648fb4d121b/bin/propolis-server/src/lib/serial/mod.rs#L109-L130

cur_output is only set to None if this future is the one chosen by select!: https://github.com/oxidecomputer/propolis/blob/1b34075ee5dd4235dc7eb46061e1e648fb4d121b/bin/propolis-server/src/lib/serial/mod.rs#L226-L230

But if this branch is not chosen, for_each_concurrent may have sent the current output bytes to some listeners and not to others. (The underlying websocket send may not be cancel-safe either but I haven't looked at its implementation.) This will cause the same data to be re-sent in the next loop iteration.

An approach along the lines suggested in RFD 400 section 5.2 might help here (and other approaches may work too):

  • If there's already a send-to-clients future active, try to resolve it
  • Else if there's no such future but there is some data waiting to be sent, create a new future to try to send that data
  • Else try to read more data from the guest

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in bin/propolis-server/src/lib/serial/mod.rs around lines 109-130 and 226-230, then read RFD 400 section 5.2. Trace how select! and for_each_concurrent handle cur_output cancellation. Done means an interrupted send cannot cause already-delivered serial data to be sent again to listeners.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.