Skip to content

Bound-check guest buffer lengths#49

Merged
jserv merged 1 commit intomasterfrom
bound-check
Apr 29, 2026
Merged

Bound-check guest buffer lengths#49
jserv merged 1 commit intomasterfrom
bound-check

Conversation

@jserv
Copy link
Copy Markdown
Contributor

@jserv jserv commented Apr 29, 2026

vm_guest_to_host validates only the start address, so a guest descriptor whose addr sits near the top of RAM with a length running off the end yields a host-side OOB read/write. Add vm_guest_buf(v, guest, len) that checks guest+len <= RAM_BASE+RAM_SIZE with a __builtin_add_overflow guard, returning NULL otherwise, and migrate every length-driven access in the virtio fast paths to it.

virtio-blk also gains a backing-store bounds check and overflow guards on the sector arithmetic. sector*512 + data_size must fit in diskimg->size; both __builtin_mul_overflow on the shift and __builtin_add_overflow on the addition reject malicious offsets before issuing I/O. Short reads/writes from diskimg_read/diskimg_write now map to VIRTIO_BLK_S_IOERR rather than the previous "io_bytes < 0" check that silently reported a stale guest buffer as success. On error, used.len reports only the 1-byte status so the guest does not consume the unwritten tail. The status descriptor is now required to advertise at least one device-writable byte before the device clobbers it.

virtio-net rx and tx snapshot desc->len into a local before the bounds check, then use only that snapshot for the read/writev byte count. The descriptor lives in guest-writable memory, so a concurrent guest write could otherwise enlarge len after the check and walk the host kernel past the validated guest range.


Summary by cubic

Add length-checked guest buffer validation across virtio paths to prevent host out-of-bounds access. Tightens virtio-blk bounds and status handling and snapshots virtio-net descriptor lengths to block TOCTOU.

  • Bug Fixes
    • Added vm_guest_buf(v, guest, len): checks guest+len within RAM using overflow guard; returns NULL on failure.
    • Replaced length-driven vm_guest_to_host calls in virtio-blk/net with vm_guest_buf.
    • virtio-blk: validate sector*512 + data_size with overflow checks and ensure end ≤ disk size; short reads/writes now return VIRTIO_BLK_S_IOERR; used.len reports only the 1-byte status on error; require status descriptor len ≥ 1.
    • virtio-net rx/tx: snapshot desc->len before bounds check and use only that value for I/O to prevent TOCTOU; disable queue on invalid buffers.

Written for commit 33b400e. Summary will update on new commits. Review in cubic

vm_guest_to_host validates only the start address, so a guest descriptor
whose addr sits near the top of RAM with a length running off the end
yields a host-side OOB read/write. Add vm_guest_buf(v, guest, len) that
checks guest+len <= RAM_BASE+RAM_SIZE with a __builtin_add_overflow
guard, returning NULL otherwise, and migrate every length-driven access
in the virtio fast paths to it.

virtio-blk also gains a backing-store bounds check and overflow guards
on the sector arithmetic. sector*512 + data_size must fit in
diskimg->size; both __builtin_mul_overflow on the shift and
__builtin_add_overflow on the addition reject malicious offsets before
issuing I/O. Short reads/writes from diskimg_read/diskimg_write now map
to VIRTIO_BLK_S_IOERR rather than the previous "io_bytes < 0" check
that silently reported a stale guest buffer as success. On error,
used.len reports only the 1-byte status so the guest does not consume
the unwritten tail. The status descriptor is now required to advertise
at least one device-writable byte before the device clobbers it.

virtio-net rx and tx snapshot desc->len into a local before the bounds
check, then use only that snapshot for the read/writev byte count. The
descriptor lives in guest-writable memory, so a concurrent guest write
could otherwise enlarge len after the check and walk the host kernel
past the validated guest range.
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 4 files

@jserv jserv merged commit a221b63 into master Apr 29, 2026
11 checks passed
@jserv jserv deleted the bound-check branch April 29, 2026 03:16
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.

1 participant