oxidecomputer / oxidecomputer/propolis

PciVirtioSock::pause maybe can hang if guests are too evil?

Open
#1,111 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

I was double-checking my understanding of the 1110-relevant bits and happened to notice this. it's "if a guest driver behaves badly enough it'll hang a thread and prevent propolis from stopping in an orderly way" which is annoying but eventually sled-agent will come clean things up, so it's not the worst.

the relevant bit is actually VsockPollerNotify. unlike most pause()s, VsockPollerNotify::pause is a Result. the error cases here are "whatever port_send(3C) might do". then over in PciVirtioSock::pause we'll call this pause, and then wait_stopped(). but if port_send fails we might not have actually sent the notification to ask the poller to stop, so we'll never finish wait_stopped(), and we'll never finish pausing devices (assuming the typical case of trying to pause all devices).

I always get itchy about EINTR but I kind of expect that EAGAIN is the more likely case here; imagine a guest that's notified the TX queue 4096 times (or just in a loop) right as we go to stop a VM. I don't think anyone could actually see this in practice.

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

Read VsockPollerNotify::pause in lib/propolis/src/vsock/poller.rs and PciVirtioSock::pause in lib/propolis/src/hw/virtio/vsock.rs, focusing on the port_send error path and the subsequent wait_stopped call. Reproduce or reason about notification failure during pause, then establish behavior that cannot leave device pausing waiting indefinitely when the poller was not notified.

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
Quiet
Clarity
Needs clarification
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.