Skip to content

fix(simt): soundness and correctness fixes for the cuda-oxide bridge and VMM - #1415

Open
elibol wants to merge 1 commit into
mainfrom
fix/audit-simt-and-vmm
Open

elibol wants to merge 1 commit into
mainfrom
fix/audit-simt-and-vmm

Conversation

@elibol

@elibol elibol commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Transplant of NVlabs/cutile-rs#257 (NVlabs/cutile-rs#257) onto cuda-rust after the repository move: one commit, original author and sign-off, host-crate paths following the move to the repository root.

Audit follow-up for the simt compatibility surface in cuda-core and cuda-async and for both VMM modules. Separate from the host-runtime patch because it changes cuda-oxide-facing APIs and is not part of 0.3.1.

Soundness

  • Synchronize wrappers reported the recorded (sticky) context error before the driver wait, so DeviceBuffer copies returned Err while the DMA still wrote the caller's host memory (reproduced). CudaStream, CudaContext, and CudaEvent::synchronize now wait first and report the sticky error after; DeviceBuffer binds and drains the sticky state before enqueueing, and leaks host memory rather than free it when the driver wait itself fails.
  • CudaStream::launch_host_function requires F: 'static, reclaims the box when the enqueue fails, and documents the callback rules.
  • cuda_async::simt::Empty and Value derive Send from their fields instead of an unconditional unsafe impl Send.
  • The execute-once unzip halves replace their unsynchronized UnsafeCell check-then-act with a mutex/condvar ExecuteOnce; a consumer on another stream waits on an event the producer records.
  • ConstantHandle carries its symbol size and an Arc<CudaModule>, rejects oversize writes and foreign-context streams, and is no longer Copy.
  • DeviceCopy for raw pointers excludes unsized pointees.
  • Mapping<'a> borrows its VirtualReservation and its PhysicalAllocation or MulticastObject, and MulticastBinding<'a> borrows its MulticastObject, in both VMM modules. Unmap is documented as not stream-ordered.

Correctness

  • DeviceBuffer constructors bind the stream's context; drop paths release resources regardless of a sticky error; the thread-local device map survives failed lookups and re-entrant sync; SingleStream errors instead of panicking before init; a colliding kernel-cache insert keeps the existing entry; simt::vmm::align_size uses checked arithmetic for any granularity; streams are counted only after creation succeeds; PinnedHostBuffer checks host pointer alignment.

Breaking for cuda-oxide
ConstantHandle::from_raw(module, dptr, size) and write_blocking(src, len); Mapping::new(&reservation, va_offset, &phys, phys_offset, size) and new_multicast(...) with the new lifetimes; launch_host_function F: 'static; Empty F: Send.

Verification on an RTX 5090 with CUDA 13.3: fmt, build, clippy with no new warnings, the CPU test script, and the GPU test script. New GPU suites simt_module_globals and simt_execute_once, plus new cases in simt_device_buffer, simt_device_buffer_leaks, simt_device_copy_impls, vmm, and simt_vmm_p2p; the sticky-error and leak tests fail without the fixes. simt_device_buffer is serialized to avoid a pre-existing driver-level deadlock between a gated host callback and concurrent cuMemFree/cuStreamDestroy.

…and VMM

Audit follow-up for the `simt` compatibility surface in cuda-core and
cuda-async and for both VMM modules. Kept separate from the host-runtime
patch because it changes cuda-oxide-facing APIs and is not part of 0.3.1.

Soundness
- Synchronize wrappers reported the recorded (sticky) context error before
  the driver wait, so DeviceBuffer copies returned Err while the DMA still
  wrote the caller's host memory (reproduced). CudaStream, CudaContext and
  CudaEvent::synchronize now wait first and report the sticky error after;
  DeviceBuffer binds and drains the sticky state before enqueueing, and
  leaks host memory rather than free it when the driver wait itself fails.
- CudaStream::launch_host_function requires F: 'static (a closure borrowing
  a local compiled and ran after its frame was gone), reclaims the box when
  the enqueue fails, and documents the callback rules.
- cuda_async::simt::Empty and Value derive Send from their fields; the
  unconditional `unsafe impl Send` laundered !Send closures.
- The execute-once unzip halves (SelectLeft/SelectRight) replace their
  unsynchronized UnsafeCell check-then-act with a mutex/condvar ExecuteOnce;
  a consumer on another stream waits on an event the producer records.
- ConstantHandle carries its symbol size and an Arc<CudaModule>; oversize
  writes and foreign-context streams are rejected; it is no longer Copy.
- DeviceCopy for raw pointers excludes unsized pointees (a wide pointer
  read back from device bytes carried an invalid vtable).
- Mapping<'a> borrows its VirtualReservation and its PhysicalAllocation or
  MulticastObject, and MulticastBinding<'a> borrows its MulticastObject, in
  both cuda-core/src/vmm.rs and simt/vmm.rs; unmap is documented as not
  stream-ordered.

Correctness
- DeviceBuffer constructors bind the stream's context; they failed on an
  unbound thread and could allocate in a foreign context.
- Drop paths release resources regardless of a sticky error; async buffers
  leaked while an error was recorded.
- The thread-local device map survives failed lookups and re-entrant sync;
  SingleStream reports use-before-init instead of panicking; a colliding
  kernel-cache insert keeps the existing entry.
- simt::vmm::align_size handles any granularity with checked arithmetic;
  streams are counted only after cuStreamCreate succeeds; PinnedHostBuffer
  checks the host pointer's alignment for T.

Docs: stale "opaque CUlaunchAttribute_st" comments, the simt collision
list, embedded-module docs, the CoordinateRequirement re-export, and the
cuda-core README's description of the two host models.

Breaking for cuda-oxide: ConstantHandle::from_raw(module, dptr, size) and
write_blocking(src, len); Mapping::new(&reservation, va_offset, &phys,
phys_offset, size) and new_multicast(...) with the new lifetimes;
launch_host_function F: 'static; Empty F: Send.

Tests: new GPU suites simt_module_globals and simt_execute_once, and new
cases in simt_device_buffer, simt_device_buffer_leaks,
simt_device_copy_impls, vmm and simt_vmm_p2p; the sticky-error and leak
tests fail without the fixes. The simt_device_buffer file is serialized
to avoid a pre-existing driver-level deadlock between a gated host
callback and concurrent cuMemFree/cuStreamDestroy.

Signed-off-by: Melih Elibol <elibol@users.noreply.github.com>
@elibol elibol added post-migration-pr cutile-rs Tile programming model: cutile crates, compiler, macros, IR, kernels, and book labels Oct 8, 2026

This branch has not been deployed

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

Labels

cutile-rs Tile programming model: cutile crates, compiler, macros, IR, kernels, and book post-migration-pr

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants