description: "Safety audit — unsafe blocks, memory leaks, undefined behavior, Vulkan spec violations"
Safety Audit
Read _audit-common.md (layout, methodology, dedup, report format) and
_audit-severity.md (the unified scale + the Special-Rules table this domain
leans on heavily) before starting. Do not restate their content here.
Severity anchors for this domain (from _audit-severity.md):
FFI lifetime violation = CRITICAL · BLAS/TLAS wrong geometry/address or
SSBO index mismatch = CRITICAL · leak that compounds per frame = HIGH ·
Vulkan spec violation = HIGH · unsafe without a safety comment = MEDIUM.
Scale of the surface
unsafe is concentrated, not scattered: ~760 occurrences live in
crates/renderer/src (ash FFI + gpu-allocator), then a long tail —
~11 each in crates/nif and crates/fsr3-sys (the vendored FSR 3.1 FFI,
Dimension 1), ~6 in crates/core, 2 in byroredux, and one each in
crates/plugin, crates/facegen, crates/cxx-bridge, crates/ui, and
crates/pex (the M47.2 decompiler — its single unsafe is a guarded
transmute in opcode.rs, see Dimension 2). crates/save (M45) and
crates/hkx (M47.2, a deliberately safe packfile reader) have no unsafe.
Counts drift — recount with grep -ro unsafe crates/<c>/src | wc -l rather
than trusting these figures. Renderer carries roughly nine SAFETY comments
per ten unsafe tokens; the residual gap is where the unsafe-without-comment
(MEDIUM) findings live. Budget your time accordingly — do not audit the
nif/core/pex tail at the expense of the renderer FFI mass. The Dimension-4
sweep greps all of crates/ so pex / save / fsr3-sys / hkx are covered
automatically.
Dimensions below are ordered by safety blast radius: FFI lifetime, then memory-corruption/UB, then per-frame leaks, then unsafe-block discipline, then Vulkan-spec compliance, then the narrower regression-guard surfaces.
Dimensions
1. FFI Lifetime Safety (cxx bridge) — CRITICAL class
- The cxx surface is currently a placeholder.
crates/cxx-bridge/src/lib.rsexposes one bridge fn,native_hello() -> String(impl incrates/cxx-bridge/cpp/native_utils.cpp). There is no raw-pointer exchange, no Rust-string-into-C++ borrow, no shared-ownership handoff across the boundary today. Do NOT report speculative "string lifetime / dangling pointer across cxx" findings against this crate — they describe a surface that does not exist yet. The real check here is a scope guard: confirm the bridge still has no owned-pointer / borrowed-slice signatures. The instant a*const,&[u8],Box<…>, orunsafe extern "C++"fn taking a Rust reference appears, this becomes a live CRITICAL-class dimension and the lifetime analysis from_audit-severityapplies. unsafe extern "C++"in the bridge marks the C++ side as trusted — verify no new fn returns a pointer Rust then dereferences past the call.crates/fsr3-sys(added 2026-07-22) is a real, live FFI crossing — unlike the cxx-bridge placeholder above, this is not hypothetical.extern "C"functions take*mut RawContext/*const RawCreateDesc/*mut RawVersionetc.;pub unsafe fn Context::create/Context::dispatchcarry# Safetydoc sections stating caller contracts (device/physical-device/proc-addr must outlive theContext; dispatch handles must belong to the creating device), andDropcalls back into the native shim. Audit everyunsafe fnhere for a# Safetydoc and a lifetime contract the way this dimension used to reserve for a hypothetical live cxx-bridge.
2. Memory Corruption / UB
- ECS cached-pointer contract (regression guard, #35 + #1367).
World::get(crates/core/src/ecs/world.rs) returns aComponentRef<'_, T>, NOT a raw pointer with a dropped guard (the unsound #35 pattern).ComponentRef,StorageRef, andStorageRefMutincrates/core/src/ecs/query.rscache a*const T/*mut Tresolved once innew()and deref it in the hot path (#1367). Each cached-derefunsafeblock carries a SAFETY comment tying the pointer's validity to the lock guard the wrapper pins. The invariant: the guard must outlive every deref, and&mut selfmust gate&mut *self.storage. Verify the SAFETY comments still match the field layout and that no refactor let a guard drop before its pointer (use-after-free → CRITICAL). #[repr(C)]GPU-struct soundness (crates/renderer/src/vulkan/scene_buffer/gpu_types.rs):GpuInstance/GpuCamera/GpuLightetc. are uploaded byte-for-byte to SSBOs. vec3 must be three scalarf32, never[f32; 3](std430 vec3 padding). A layout drift here is silent per-instance corruption — see Dimension 6 for the GpuMaterial pin and_audit-severity's#[repr(C)]-drift HIGH row.- NIF bulk POD reads (
NifStream::read_pod_vec,crates/nif/src/stream.rs; the header mirrorread_pod_vec_from_cursor,crates/nif/src/header.rs):read_exactof raw LE bytes into aT: AnyBitPatternvector. SAFETY comments must hold —Tis restricted to bit-pattern-safe types (a sealed bound stopsread_pod_vec::<bool>). Verify the byte-count overflow guard (count × size) is present and no caller widensTpastAnyBitPattern. - sfmaterial enum decode (
BuiltinType::from_u32,crates/sfmaterial/src/types.rs): MUST stay a checkedmatchover the0xFFFFFF##tags with a_ => return Err(Error::UnsupportedBuiltin { raw })arm (confirmed present). The module doc's "transmute into this enum" wording is aspirational prose, NOT the impl — an actualstd::mem::transmuteof an unmatched#[repr(u32)]byte pattern is UB. Verify thematch+Errarm survive any "optimization." - pex opcode decode (
OpCode::from_u8,crates/pex/src/opcode.rs): unlike sfmaterial, this one IS a realunsafe { std::mem::transmute::<u8, OpCode>(byte) }— sound ONLY because the SAFETY comment's two preconditions hold:OpCodeis#[repr(u8)]with contiguous discriminants0..MAX_OPCODE, ANDbyteis range-checked (< MAX_OPCODE) before the transmute. Both must stay true: a gap in the discriminant sequence, or a refactor that drops the bound check, makes an out-of-range byte UB. Verify the guard and the contiguity (no skipped values in the enum) on any opcode-table change. - Stack-overflow risk: no unbounded recursion in block-walk / scene-graph traversal.
3. Memory & Resource Leaks (HIGH when per-frame/per-cell)
- Rapier bodies on cell unload (regression guard, #1520,
34c7a218).crates/physics/src/world.rs::remove_*andbyroredux/src/cell_loader/unload.rsmust release a cell's rigid bodies, colliders, and impulse joints fromRigidBodySet/ColliderSet/ImpulseJointSet(plus broad-phase / query-pipeline state) when the cell unloads. Without it they accumulate per cell — a steady leak under exterior streaming. Guard test:byroredux/src/cell_loader/rapier_release_tests.rs. Verify the release path is still wired and the test still asserts emptiness post-unload. - Deferred-destroy drain (
crates/renderer/src/deferred_destroy.rs,DeferredDestroyQueue<T>shared by mesh + BLAS + BLAS-scratch buffer (#1782) + texture + skin compute): objects are destroyed only after the in-flight fence clears (#418 moved the tick after fence wait; #732 added an explicit shutdown drain). Verify the tick still runs after fence wait incontext/draw.rsand the shutdown sweep drains the queue — a missed drain leaks GPU memory across the app lifetime, a too-early destroy is use-after-free (CRITICAL). AllocatorResourcedrop ordering (regression guard, #1406,299e6a84).AllocatorResource(crates/renderer/src/vulkan/allocator.rs; held inbyroredux/src/main.rs) must be removed from the ECSWorldBEFOREVulkanContext::drop()runs. The allocator holds a liveArc<Device>; if theWorldoutlives the context, the allocator'sDropcalls the driver against a destroyed logical device (use-after-free → CRITICAL). Verify the main loop removes the resource before dropping the renderer, including the panic-unwind path that could skip the removal.- GPU allocation inventory — every long-lived allocation tracked and freed:
BLAS scratch/result, TLAS instance/result, G-buffer images, SVGF history, TAA
per-FIF history images, caustic + water-caustic R32_UINT accumulators
(
caustic.rs/water_caustic.rs), per-skinned-entity SkinSlot output buffers, MaterialBuffer SSBO, volumetric/bloom mip pyramids. Cross-check eviction thresholds againstdocs/engine/memory-budget.md; do not re-derive. - CPU-side unbounded growth —
Vec/HashMapkeyed by cell or path that never shrinks. The MaterialTable dedup map and AnimationClipRegistry (Dimension 8) are the known per-cell-growth risks.
4. Unsafe-Block Discipline (MEDIUM — the bread-and-butter sweep)
- Grep every
unsafeincrates/+byroredux/(.rs). For each: is there a SAFETY comment, and does the comment's stated invariant actually hold at this call site? A correct unsafe block with no comment is still a MEDIUM finding (_audit-severitySpecial Rules). A commented block whose invariant is FALSE is the higher-severity finding. - Heaviest in
crates/renderer/src/vulkan/ash FFI — the SAFETY/unsafe count gap (~676 vs ~761) is the haystack. Spot-check the ash dispatch wrappers, the gpu-allocatorArc<Mutex<…>>interactions, and anyfrom_raw_parts/caston mapped memory. - Report unsafe blocks lacking comments as a batched MEDIUM finding (list the sites) rather than one finding per block, unless an invariant is actually unsound.
5. Vulkan Spec Compliance (HIGH — but flag what cargo test can't see)
Per the No-Speculative-Vulkan-Fixes rule: render-pass / barrier / pipeline-state spec claims that are invisible to
cargo testMUST be framed as "needs validation-layer or RenderDoc verification", not asserted as confirmed bugs. Run the engine with validation layers (debug build) and report ANY emitted error verbatim — that is the sound evidence channel for this dimension.
- All
vkCreate*/vkDestroy*paired; Drop ordering destroys children before parents (device-destroy is last). - Queue submission ordering: wait-before-signal; per-image semaphores.
- Acceleration structures (
crates/renderer/src/vulkan/acceleration/): correct geometry flags, valid device addresses, buffers carrySHADER_DEVICE_ADDRESS. TLAS UPDATE mode — instance/geometry count must match the original BUILD. Skin BLAS refit — vertex/geometry count must match BUILD; a bone-count change forces a full rebuild. (Wrong AS geometry/address = CRITICAL per_audit-severity.) - TLAS resize wait (regression guard, #1390,
a7e1502b). The resize branch inacceleration/tlas.rscallsdevice.device_wait_idle()before freeing the old allocation (confirmed present). Verify the wait survives — without it the GPU may still consume the old TLAS scratch during free under a resize-under-load refactor. VK_KHR_ray_queryenabled + feature-gated before any ray-query use.- Per-frame compute layout hygiene (TAA / caustic / water-caustic / volumetrics /
bloom): images that coexist as storage-write + sampled-read are held in
GENERAL;initialize_layoutsdoes the one-time UNDEFINED→GENERAL transition for every mip / FIF slot. A missed slot is an UNDEFINED-read validation error. CLEAR-before- COMPUTE invariant (caustic R32_UINTimageAtomicAdd, volumetric inject) — a missing clear is persistent cross-frame ghost accumulation. Verify the volumetrics caller honors the dispatch gate:VOLUMETRIC_OUTPUT_CONSUMED(crates/renderer/src/vulkan/volumetrics.rs) is nowtrue, so the pass is live — dispatch is dead only while it readsfalse. Callers MUST gatevol.dispatch()on that const either way (context/post_passes.rs); read the const rather than assuming a state. - SPIR-V reflection (
crates/renderer/src/vulkan/reflect.rs): the Rust descriptor layout must match shader-declared bindings — this is the one binding-drift check that IS visible tocargo test(scene_descriptor_reflection_tests). Prefer it over eyeballing descriptor writes.
6. R1 Material Table Layout Soundness
GpuMaterialsize is pinned at 348 B bygpu_material_size_is_348_bytes(crates/renderer/src/vulkan/material.rs) — the test name now matches the asserted size (history: 272 → 260 after #804 droppedavg_albedo, → 296 with the Disney sheen/subsurface lobe #1249, → 300 withanisotropic#1250, → 348 on 2026-07-27 (1d94eb24) with the twelve common supplemental texture roles). A stale 260/272/296/300 in audit prose, or any test-name-vs-asserted-size mismatch, means the GPU is reading wrong bytes.- Per-field offset pin
gpu_material_field_offsets_match_shader_contract(#806): every named field's byte offset asserted against the shader contract. The size pin alone cannot catch a within-vec4 reorder (swaptexture_index ↔ normal_map_indexis size-invisible, runtime-lethal). Adding a field without updating this assertion is a regression. - ALL fields are flat scalar
f32/u32— never[f32; 3](std430 vec3 alignment). This includes the newest scalars: the BGSM translucency suite (translucency_subsurface_r/g/b,…_transmissive_scale,…_turbulence) and the Disney lobe (ior,subsurface,sheen,sheen_tint,anisotropic). - Pad fields explicitly zeroed (the byte-
Hash/Eqdedup hashes the raw 348 B; an uninit hole poisons dedup). New scalars must be zeroed inGpuMaterial::default()so default materials still dedup to slot 0. - Intern cap (#797).
MaterialTable::interncaps atMAX_MATERIALS = 16384(scene_buffer/constants.rs); over-cap interns return id0with a one-shot warn — no SSBO over-index, no DEVICE_LOST.upload_materials(scene_buffer/upload.rs)debug_assertslen <= MAX_MATERIALSand clamps with.min(MAX_MATERIALS). Verify the intern cap and the upload truncation stay in lockstep. GpuInstance.material_idindexes the SSBO with NO GPU bounds check — CPU must guarantee in-range (SSBO index mismatch = CRITICAL).ui.vertMaterialBuffer read offsets must stay in lockstep with the canonicalstruct GpuMaterial/GpuInstanceincrates/renderer/shaders/include/bindings.glsl(triangle.frag#includes it) — #785 was a stale-hunk regression reading wrong bytes — nameui.vertexplicitly.
7. RT IOR-Refraction Safety (regression guards)
- Glass-passthrough loop guard (#789): the texture-equality identity check at the refraction hit prevents unbounded recursion when coincident glass surfaces share an albedo/normal-map descriptor pair. A regression is a frame-time hang on any paired-glass cell. Verify the check is present.
- Glass ray budget
GLASS_RAY_BUDGET(crates/renderer/src/shader_constants_data.rs, mirrored incrates/renderer/shaders/include/shader_constants.glsl— verify the two stay in lockstep; raised from 8192 in6efe1706, and again since — check the constant by name rather than trusting a hard-coded figure here). It is a runaway-recursion cap, not a quality knob. #1438 documented that the atomicAdd accounting can overshoot the budget unconditionally — note that nuance rather than reporting the overshoot as new. Verify the budget is enforced at every glass call site. - Frisvad orthonormal basis (#820): the naive
cross(N, world-up)basis degenerates near-vertical (zero-length → NaN ray). Verify Frisvad is the active path for IOR refraction roughness spread. - IOR miss fallback for interiors uses cell-ambient, not global sky tint (open-sky leakage into dungeons is a visible regression).
DBG_VIZ_GLASS_PASSTHRU = 0x80is a permanent diagnostic bit — verify it hasn't collided with a new debug flag (full catalog incrates/renderer/src/shader_constants_data.rs, mirrored to the generatedcrates/renderer/shaders/include/shader_constants.glsl).
8. NPC / Animation Spawn Safety
- B-spline pose-fallback sentinel (#772): NPCs vanishing under FNV
BSPSysSimpleColorModifierparticle stacks sharing keyframe time-zero with the actor's player must be gated on anFLT_MAXsentinel. Removing the gate is whole-NPC disappearance, not a stuck pose. Verify the sentinel is wired. - AnimationClipRegistry dedup (#790): the registry interns by lowercased path so cell streaming doesn't grow it unboundedly (otherwise one keyframe set leaks per cell load → steady RAM growth). Verify case-insensitive interning is preserved.
- B-splines reach FNV / FO3 too (
feedback_bspline_not_skyrim_only.md) — do NOT rule outNiBSplineCompTransformInterpolatorby game era. - Starfield content is WALKABLE (Cydonia) — SF cells reach the spawn/animation path; don't short-circuit spawn-safety reasoning with "no SF content exercises this."
MAX_TOTAL_BONESoverflow guard must fire — silent truncation past cap was the M29 regression. The slot-exhaustion warn lives onSkinSlotPool(crates/core/src/ecs/resources/skin_slot_pool.rs), one-shot via theoverflow_warnedflag withoverflow_attempt_countcarrying total demand; excess skinned entities fall back to bind pose rather than over-indexing. Guard tests:byroredux/src/render/bone_palette_overflow_tests.rs.
9. NIFAL Boundary — NaN/Inf on the GPU (UB facet only)
See /audit-nifal for correctness-of-mapping; this dimension covers ONLY the
safety facet — NaN/inf scalars reaching the GPU, unbounded allocation.
byroredux/src/material_translate.rs::translate_materialdeliberately seedsf32::NANintoMaterial.metalness/roughness(mesh.metalness_override.unwrap_or(f32::NAN), same for roughness).Material::resolve_pbr(crates/core/src/ecs/components/material.rs) is the ONLY thing that detects (is_nan()) and clamps these sentinels before they reachGpuMaterial. Both fields are now plainf32(noOption), so a producer that skipsresolve_pbr()ships a NaN into the SSBO silently (NaN-on-GPU = UB). Verify EVERY renderer-boundMaterialproducer runsresolve_pbr()or constructs already-finite values (thestatic_meshes.rsfallback constructs finite defaults directly — confirm it still does).- Collision translate (
crates/nif/src/import/collision/mod.rs, coversBhkMultiSphereShape+BhkConvexListShape): emitted half-extents / radii / sphere centers must be finite and bounded — a NaN/inf shape param propagates into the physics solver and the BLAS build. - Typed particle blocks (
crates/nif/src/blocks/particle.rs) →extract_emitter_params/extract_emitter_rate(crates/nif/src/import/walk/mod.rs) →apply_emitter_params(byroredux/src/systems/particle.rs): emitter rate / lifespan / size must be finite and non-negative at the extract boundary — an unbounded or NaN rate is an unbounded-allocation / NaN-transform risk downstream.
10. debug-ui (egui overlay) Teardown & Shared-Allocator Safety
crates/debug-ui/src/lib.rsDebugUiStateis the CPU half only — egui context,egui_winitstate, lastFullOutput, panel state. It holds no Vulkan handle. It lives as an ECS resource (impl Resource for DebugUiState) and is owned by the main loop.- The Vulkan half is
EguiPass(crates/renderer/src/vulkan/egui_pass.rs, held asVulkanContext::egui_pass: Option<EguiPass>). It takes anash::Device+ the sharedArc<Mutex<gpu_allocator::vulkan::Allocator>>and wrapsegui-ash-renderer, which owns its own descriptor pool + per-texture images, plusEguiPass's ownvk::RenderPass+ per-swapchain-image framebuffers. Those MUST be freed before the engine destroys theash::Device— same class as Dimension 3's allocator-before-device rule. Verify theOption<EguiPass>teardown runs ahead ofVulkanContext's device-destroy. - Texture free is deferred one frame (
pending_free), leaning ondraw_frame's fence wait — verify the defer survives; freeing on the arriving frame is a use-after-free. - The allocator mutex is SHARED with the render thread — verify it is held for
minimum duration during egui texture upload; a long hold stalls rendering. The
graphics queue is likewise passed as a
Mutex(EguiDispatchCtx::queue) so the lock scopes to theset_texturessubmit only (CONC-D1-01 / #1713) — a widened hold regresses that.
Procedure
- Grep all
unsafeincrates/+byroredux/(.rs); note the renderer mass and the SAFETY-comment gap (Dimension 4). - Confirm the cxx bridge is still a no-pointer placeholder (Dimension 1).
- Audit the cached-pointer ECS contract + repr(C) GPU structs + NIF POD reads + sfmaterial decode (Dimension 2).
- Walk the leak inventory and the three drop-ordering regression guards — Rapier release, deferred-destroy drain, AllocatorResource removal (Dimension 3).
- Sweep unsafe-block discipline; batch the comment-less blocks (Dimension 4).
- Vulkan-spec pass — run validation layers, report emitted errors verbatim; frame barrier/layout claims invisible to cargo test as "needs RenderDoc verification" (Dimension 5).
- R1 material layout pins (Dimension 6), IOR/glass guards (7), NPC/anim spawn (8), NIFAL NaN boundary (9), debug-ui teardown (10).
- Dedup against open/closed issues (
_audit-commonDeduplication) — most items above are regression guards; recast a confirmed-intact guard as PASS, not a NEW finding. - Save the report to
docs/audits/AUDIT_SAFETY_<TODAY>.md(see_audit-commonReport Finalization).
Next.js App Router Expert
Development
A skill that turns Claude into a Next.js App Router expert.
README Generator
Development
Creates professional and comprehensive README.md files for your projects.
API Documentation Writer
Development
Generates comprehensive API documentation in OpenAPI/Swagger format.