Skip to content

Commit da8e8b5

Browse files
paolobarboliniMoritz Borcherding
paolobarbolini
authored and
Moritz Borcherding
committed
Fix RingBuffer::extend_from_within_unchecked unsoundness (#76)
1 parent b99a8b9 commit da8e8b5

File tree

1 file changed

+151
-31
lines changed

1 file changed

+151
-31
lines changed

‎src/decoding/ringbuffer.rs

Lines changed: 151 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -295,50 +295,170 @@ impl RingBuffer {
295295
/// 2. More then len reserved space so we do not write out-of-bounds
296296
#[warn(unsafe_op_in_unsafe_fn)]
297297
pub unsafe fn extend_from_within_unchecked(&mut self, start: usize, len: usize) {
298+
debug_assert!(start + len <= self.len());
299+
debug_assert!(self.free() >= len);
300+
298301
if self.head < self.tail {
299-
// continous data slice |____HDDDDDDDT_____|
302+
// Continuous source section and possibly non continuous write section:
303+
//
304+
// H T
305+
// Read: ____XXXXSSSSXXXX________
306+
// Write: ________________DDDD____
307+
//
308+
// H: Head position (first readable byte)
309+
// T: Tail position (first writable byte)
310+
// X: Uninvolved bytes in the readable section
311+
// S: Source bytes, to be copied to D bytes
312+
// D: Destination bytes, going to be copied from S bytes
313+
// _: Uninvolved bytes in the writable section
300314
let after_tail = usize::min(len, self.cap - self.tail);
301-
unsafe {
315+
316+
let src = (
317+
// SAFETY: `len <= isize::MAX` and fits the memory range of `buf`
318+
unsafe { self.buf.as_ptr().add(self.head + start) }.cast_const(),
319+
// Src length (see above diagram)
320+
self.tail - self.head - start,
321+
);
322+
323+
let dst = (
324+
// SAFETY: `len <= isize::MAX` and fits the memory range of `buf`
325+
unsafe { self.buf.as_ptr().add(self.tail) },
326+
// Dst length (see above diagram)
327+
self.cap - self.tail,
328+
);
329+
330+
// SAFETY: `src` points at initialized data, `dst` points to writable memory
331+
// and does not overlap `src`.
332+
unsafe { copy_bytes_overshooting(src, dst, after_tail) }
333+
334+
if after_tail < len {
335+
// The write section was not continuous:
336+
//
337+
// H T
338+
// Read: ____XXXXSSSSXXXX__
339+
// Write: DD______________DD
340+
//
341+
// H: Head position (first readable byte)
342+
// T: Tail position (first writable byte)
343+
// X: Uninvolved bytes in the readable section
344+
// S: Source bytes, to be copied to D bytes
345+
// D: Destination bytes, going to be copied from S bytes
346+
// _: Uninvolved bytes in the writable section
347+
302348
let src = (
303-
self.buf.as_ptr().cast_const().add(self.head + start),
304-
self.tail - self.head,
349+
// SAFETY: we are still within the memory range of `buf`
350+
unsafe { src.0.add(after_tail) },
351+
// Src length (see above diagram)
352+
src.1 - after_tail,
353+
);
354+
let dst = (
355+
self.buf.as_ptr(),
356+
// Dst length overflowing (see above diagram)
357+
self.head,
305358
);
306-
let dst = (self.buf.as_ptr().add(self.tail), self.cap - self.tail);
307-
copy_bytes_overshooting(src, dst, after_tail);
308359

309-
if after_tail < len {
310-
let src = (src.0.add(after_tail), src.1 - after_tail);
311-
let dst = (self.buf.as_ptr(), self.head);
312-
copy_bytes_overshooting(src, dst, len - after_tail);
313-
}
360+
// SAFETY: `src` points at initialized data, `dst` points to writable memory
361+
// and does not overlap `src`.
362+
unsafe { copy_bytes_overshooting(src, dst, len - after_tail) }
314363
}
315364
} else {
316-
// continous free slice |DDDT_________HDDDD|
317365
if self.head + start > self.cap {
366+
// Continuous read section and destination section:
367+
//
368+
// T H
369+
// Read: XXSSSSXXXX____________XX
370+
// Write: __________DDDD__________
371+
//
372+
// H: Head position (first readable byte)
373+
// T: Tail position (first writable byte)
374+
// X: Uninvolved bytes in the readable section
375+
// S: Source bytes, to be copied to D bytes
376+
// D: Destination bytes, going to be copied from S bytes
377+
// _: Uninvolved bytes in the writable section
378+
318379
let start = (self.head + start) % self.cap;
319-
unsafe {
320-
let src = (
321-
self.buf.as_ptr().add(start).cast_const(),
322-
self.cap - self.head,
323-
);
324-
let dst = (self.buf.as_ptr().add(self.tail), self.head - self.tail);
325-
copy_bytes_overshooting(src, dst, len);
326-
}
380+
381+
let src = (
382+
// SAFETY: `len <= isize::MAX` and fits the memory range of `buf`
383+
unsafe { self.buf.as_ptr().add(start) }.cast_const(),
384+
// Src length (see above diagram)
385+
self.tail - start,
386+
);
387+
388+
let dst = (
389+
// SAFETY: `len <= isize::MAX` and fits the memory range of `buf`
390+
unsafe { self.buf.as_ptr().add(self.tail) }, // Dst length (see above diagram)
391+
// Dst length (see above diagram)
392+
self.head - self.tail,
393+
);
394+
395+
// SAFETY: `src` points at initialized data, `dst` points to writable memory
396+
// and does not overlap `src`.
397+
unsafe { copy_bytes_overshooting(src, dst, len) }
327398
} else {
399+
// Possibly non continuous read section and continuous destination section:
400+
//
401+
// T H
402+
// Read: XXXX____________XXSSSSXX
403+
// Write: ____DDDD________________
404+
//
405+
// H: Head position (first readable byte)
406+
// T: Tail position (first writable byte)
407+
// X: Uninvolved bytes in the readable section
408+
// S: Source bytes, to be copied to D bytes
409+
// D: Destination bytes, going to be copied from S bytes
410+
// _: Uninvolved bytes in the writable section
411+
328412
let after_start = usize::min(len, self.cap - self.head - start);
329-
unsafe {
413+
414+
let src = (
415+
// SAFETY: `len <= isize::MAX` and fits the memory range of `buf`
416+
unsafe { self.buf.as_ptr().add(self.head + start) }.cast_const(),
417+
// Src length - chunk 1 (see above diagram on the right)
418+
self.cap - self.head - start,
419+
);
420+
421+
let dst = (
422+
// SAFETY: `len <= isize::MAX` and fits the memory range of `buf`
423+
unsafe { self.buf.as_ptr().add(self.tail) },
424+
// Dst length (see above diagram)
425+
self.head - self.tail,
426+
);
427+
428+
// SAFETY: `src` points at initialized data, `dst` points to writable memory
429+
// and does not overlap `src`.
430+
unsafe { copy_bytes_overshooting(src, dst, after_start) }
431+
432+
if after_start < len {
433+
// The read section was not continuous:
434+
//
435+
// T H
436+
// Read: SSXXXXXX____________XXSS
437+
// Write: ________DDDD____________
438+
//
439+
// H: Head position (first readable byte)
440+
// T: Tail position (first writable byte)
441+
// X: Uninvolved bytes in the readable section
442+
// S: Source bytes, to be copied to D bytes
443+
// D: Destination bytes, going to be copied from S bytes
444+
// _: Uninvolved bytes in the writable section
445+
330446
let src = (
331-
self.buf.as_ptr().add(self.head + start).cast_const(),
332-
self.cap - self.head,
447+
self.buf.as_ptr().cast_const(),
448+
// Src length - chunk 2 (see above diagram on the left)
449+
self.tail,
333450
);
334-
let dst = (self.buf.as_ptr().add(self.tail), self.head - self.tail);
335-
copy_bytes_overshooting(src, dst, after_start);
336-
337-
if after_start < len {
338-
let src = (self.buf.as_ptr().cast_const(), self.tail);
339-
let dst = (dst.0.add(after_start), dst.1 - after_start);
340-
copy_bytes_overshooting(src, dst, len - after_start);
341-
}
451+
452+
let dst = (
453+
// SAFETY: we are still within the memory range of `buf`
454+
unsafe { dst.0.add(after_start) },
455+
// Dst length (see above diagram)
456+
dst.1 - after_start,
457+
);
458+
459+
// SAFETY: `src` points at initialized data, `dst` points to writable memory
460+
// and does not overlap `src`.
461+
unsafe { copy_bytes_overshooting(src, dst, len - after_start) }
342462
}
343463
}
344464
}

0 commit comments

Comments
 (0)