Skip to content

Commit

Permalink
fix: correct checksum for misaligned slices
Browse files Browse the repository at this point in the history
Also fix a new lint warning.
  • Loading branch information
mlegner committed May 17, 2024
1 parent 0c3aba7 commit 0bc12b2
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 22 deletions.
45 changes: 29 additions & 16 deletions crates/scion-proto/src/packet/checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,35 +118,48 @@ impl ChecksumDigest {
///
/// If the slice is not a multiple of 2-bytes, then it is zero-padded
/// before being added to the checksum.
pub fn add_slice(&mut self, data: &[u8]) -> &mut Self {
pub fn add_slice(&mut self, mut data: &[u8]) -> &mut Self {
if data.is_empty() {
return self;
}
let mut initial_sum = 0;

// Before converting to a `&[u16]` we need to make sure the slice is aligned.
let is_aligned = data.as_ptr().align_offset(2) == 0;
if !is_aligned {
// We want to zero-prepend the value, i.e., for slice where we pair the elements, we
// have [0, A], [B, C], ... Storing [0, X] on a little endian architecture gets written
// as [X, 0] to memory, so we need to swap it with `to_be()`.
initial_sum = (data[0] as u16).to_be() as u32;
data = &data[1..];
};
let ptr: *const u8 = data.as_ptr();

// Converting to a &[u16] requires an even number of elements in the slice
let (data, initial_sum) = if data.len() % 2 == 0 {
(data, 0u32)
} else {
(
&data[..data.len() - 1],
// We want to zero pad the value, i.e., for slice where we pair the elements,
// we have [A, B], [C, D], ... [X, 0]. Since all the values are currently in
// memory in the order [A, B] storing [0, X] on a little endian architecture
// gets written as [X, 0] to memory. On big-endian this would get written as
// [0, X] so we swap it only on that big-endian architectures with to_le()
(data[data.len() - 1] as u16).to_le() as u32,
)
// Converting to a `&[u16]` requires an even number of elements in the slice.
if data.len() % 2 != 0 {
// We want to zero pad the value, i.e., for slice where we pair the elements,
// we have [A, B], [C, D], ... [X, 0]. Since all the values are currently in
// memory in the order [A, B] storing [0, X] on a little endian architecture
// gets written as [X, 0] to memory. On big-endian this would get written as
// [0, X] so we swap it only on that big-endian architectures with to_le().
initial_sum += (data[data.len() - 1] as u16).to_le() as u32;
data = &data[..data.len() - 1];
};

let ptr: *const u8 = data.as_ptr();
let data_u16 = unsafe { slice::from_raw_parts(ptr as *const u16, data.len() / 2) };

let sum_with_overflow = data_u16
.iter()
.fold(initial_sum, |sum, value| sum + (*value as u32));

// Already incorporate the overflow, as it simplifies the endian conversion below
let sum = Self::fold_checksum(sum_with_overflow) as u16;
let mut sum = Self::fold_checksum(sum_with_overflow) as u16;

// If the original slice was not aligned, we need to swap the bytes to get the correct
// checksum.
if !is_aligned {
sum = sum.swap_bytes();
}

// The above sum is actually in big-endian but stored in big/little endian depending
// on the platform. If the platform is little endian, this call will swap the byte-order
Expand Down
4 changes: 1 addition & 3 deletions crates/scion-proto/src/path/standard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,7 @@ where
///
/// There are always at most 3 segments.
pub fn segment(&self, segment_index: usize) -> Option<Segment> {
let Some(info_field) = self.info_field(segment_index) else {
return None;
};
let info_field = self.info_field(segment_index)?;

// Get the index of the first hop field in the segment.
// This is equivalent to the index after all preceding hop fields.
Expand Down
4 changes: 1 addition & 3 deletions crates/scion/src/pan/path_strategy/refresher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,9 +341,7 @@ impl<'a> Iterator for PathsTo<'a> {
type Item = &'a Path;

fn next(&mut self) -> Option<Self::Item> {
let Some(paths) = self.paths else {
return None;
};
let paths = self.paths?;

#[allow(clippy::while_let_on_iterator)]
while let Some(info) = self.inner.next() {
Expand Down

0 comments on commit 0bc12b2

Please sign in to comment.