diff --git a/node-graph/libraries/core-types/src/node.rs b/node-graph/libraries/core-types/src/node.rs index 38132bf520..2c2f7ed326 100644 --- a/node-graph/libraries/core-types/src/node.rs +++ b/node-graph/libraries/core-types/src/node.rs @@ -106,9 +106,10 @@ pub struct RecordBatchMut<'a> { impl<'a> RecordBatchMut<'a> { /// Minted only by a [`crate::record::SlotRun`] finishing its served lanes, - /// which is what makes the initialized prefix a fact rather than a contract. + /// which serve in ascending order with no gaps, so the initialized prefix is + /// a fact rather than a contract. pub(crate) fn new(scratch: &'a mut [MaybeUninit], len: usize, layout: &'a crate::record::Layout) -> Self { - debug_assert!(len * layout.lane_stride() <= scratch.len() * 8); + debug_assert!(len.checked_mul(layout.lane_stride()).is_some_and(|need| need <= scratch.len() * 8)); Self { scratch, len, layout } } diff --git a/node-graph/libraries/core-types/src/record/input.rs b/node-graph/libraries/core-types/src/record/input.rs index b406432ee0..a22bec88c2 100644 --- a/node-graph/libraries/core-types/src/record/input.rs +++ b/node-graph/libraries/core-types/src/record/input.rs @@ -90,7 +90,10 @@ where let Some(len) = range.end.checked_sub(range.start).and_then(|len| usize::try_from(len).ok()) else { return BatchStatus::InvalidRange; }; - let words = len * node.layout().lane_stride() / 8; + // Checked: a wrapped product would size the scratch below the run. + let Some(words) = len.checked_mul(node.layout().lane_stride()).map(|bytes| bytes / 8) else { + return BatchStatus::InvalidRange; + }; let exhausted = || { BatchStatus::Error(crate::gpoll::GraphError { kind: crate::gpoll::ErrorKind::ArenaExhausted, diff --git a/node-graph/libraries/core-types/src/record/serve.rs b/node-graph/libraries/core-types/src/record/serve.rs index 0869472445..b67bd6f3d3 100644 --- a/node-graph/libraries/core-types/src/record/serve.rs +++ b/node-graph/libraries/core-types/src/record/serve.rs @@ -20,8 +20,13 @@ pub struct SlotRun<'a> { } impl<'a> SlotRun<'a> { + /// `None` where the scratch cannot hold `len` lanes. The products are + /// checked: the stride is a multiple of 8, so a wrapped one would pass the + /// capacity test vacuously. pub(in crate::record) fn new(scratch: &'a mut [std::mem::MaybeUninit], len: usize, layout: &'a Layout) -> Option> { - (scratch.len() * 8 >= len * layout.lane_stride()).then_some(SlotRun { scratch, layout, len, filled: 0 }) + let need = len.checked_mul(layout.lane_stride())?; + let capacity = scratch.len().checked_mul(8)?; + (capacity >= need).then_some(SlotRun { scratch, layout, len, filled: 0 }) } pub fn layout(&self) -> &'a Layout { @@ -312,6 +317,17 @@ mod tests { frames.claim(&layout).element(1u32, &arena); } + #[test] + fn a_run_refuses_a_lane_count_whose_stride_product_overflows() { + let layout = Layout::default().with_writes(0, element_write::(), &[]); + let mut scratch = [std::mem::MaybeUninit::::uninit(); 4]; + let mut frame_arena = FrameArena::new(); + frame_arena.reserve(64); + let frames = frame_arena.frames(); + let wrapping = usize::MAX / layout.lane_stride() + 1; + assert!(frames.run(&mut scratch, wrapping, &layout).is_none(), "a wrapped capacity product must not pass the check"); + } + #[test] fn a_claim_shortens_onto_a_derived_lifetime() { fn shorten<'long: 'short, 'short, 'l>(claim: FrameClaim<'long, 'l>) -> FrameClaim<'short, 'l> {