aya: Remove unnecessary unsafe markers on map iteration.

Map iteration can yield stale keys and values by virtue of sharing a
data structure with BPF programs which can modify it. However, all
accesses remain perfectly safe and will not cause memory corruption or
data races.
pull/131/head
Thia Wyrod 3 years ago
parent 07a6016ebb
commit 18970369e2
No known key found for this signature in database
GPG Key ID: 55D3AB7E5658CA0C

@ -94,7 +94,7 @@ impl<T: Deref<Target = Map>, V: Pod> Array<T, V> {
/// An iterator over the elements of the array. The iterator item type is `Result<V, /// An iterator over the elements of the array. The iterator item type is `Result<V,
/// MapError>`. /// MapError>`.
pub unsafe fn iter(&self) -> impl Iterator<Item = Result<V, MapError>> + '_ { pub fn iter(&self) -> impl Iterator<Item = Result<V, MapError>> + '_ {
(0..self.len()).map(move |i| self.get(&i, 0)) (0..self.len()).map(move |i| self.get(&i, 0))
} }
@ -134,7 +134,7 @@ impl<T: Deref<Target = Map>, V: Pod> IterableMap<u32, V> for Array<T, V> {
&self.inner &self.inner
} }
unsafe fn get(&self, index: &u32) -> Result<V, MapError> { fn get(&self, index: &u32) -> Result<V, MapError> {
self.get(index, 0) self.get(index, 0)
} }
} }

@ -113,7 +113,7 @@ impl<T: Deref<Target = Map>, V: Pod> PerCpuArray<T, V> {
/// An iterator over the elements of the array. The iterator item type is /// An iterator over the elements of the array. The iterator item type is
/// `Result<PerCpuValues<V>, MapError>`. /// `Result<PerCpuValues<V>, MapError>`.
pub unsafe fn iter(&self) -> impl Iterator<Item = Result<PerCpuValues<V>, MapError>> + '_ { pub fn iter(&self) -> impl Iterator<Item = Result<PerCpuValues<V>, MapError>> + '_ {
(0..self.len()).map(move |i| self.get(&i, 0)) (0..self.len()).map(move |i| self.get(&i, 0))
} }
@ -153,7 +153,7 @@ impl<T: Deref<Target = Map>, V: Pod> IterableMap<u32, PerCpuValues<V>> for PerCp
&self.inner &self.inner
} }
unsafe fn get(&self, index: &u32) -> Result<PerCpuValues<V>, MapError> { fn get(&self, index: &u32) -> Result<PerCpuValues<V>, MapError> {
self.get(index, 0) self.get(index, 0)
} }
} }

@ -79,7 +79,7 @@ impl<T: Deref<Target = Map>> ProgramArray<T> {
/// An iterator over the indices of the array that point to a program. The iterator item type /// An iterator over the indices of the array that point to a program. The iterator item type
/// is `Result<u32, MapError>`. /// is `Result<u32, MapError>`.
pub unsafe fn indices(&self) -> MapKeys<'_, u32> { pub fn indices(&self) -> MapKeys<'_, u32> {
MapKeys::new(&self.inner) MapKeys::new(&self.inner)
} }

@ -61,7 +61,7 @@ impl<T: Deref<Target = Map>, K: Pod, V: Pod> HashMap<T, K, V> {
} }
/// Returns a copy of the value associated with the key. /// Returns a copy of the value associated with the key.
pub unsafe fn get(&self, key: &K, flags: u64) -> Result<V, MapError> { pub fn get(&self, key: &K, flags: u64) -> Result<V, MapError> {
let fd = self.inner.deref().fd_or_err()?; let fd = self.inner.deref().fd_or_err()?;
let value = bpf_map_lookup_elem(fd, key, flags).map_err(|(code, io_error)| { let value = bpf_map_lookup_elem(fd, key, flags).map_err(|(code, io_error)| {
MapError::SyscallError { MapError::SyscallError {
@ -75,13 +75,13 @@ impl<T: Deref<Target = Map>, K: Pod, V: Pod> HashMap<T, K, V> {
/// An iterator visiting all key-value pairs in arbitrary order. The /// An iterator visiting all key-value pairs in arbitrary order. The
/// iterator item type is `Result<(K, V), MapError>`. /// iterator item type is `Result<(K, V), MapError>`.
pub unsafe fn iter(&self) -> MapIter<'_, K, V, Self> { pub fn iter(&self) -> MapIter<'_, K, V, Self> {
MapIter::new(self) MapIter::new(self)
} }
/// An iterator visiting all keys in arbitrary order. The iterator element /// An iterator visiting all keys in arbitrary order. The iterator element
/// type is `Result<K, MapError>`. /// type is `Result<K, MapError>`.
pub unsafe fn keys(&self) -> MapKeys<'_, K> { pub fn keys(&self) -> MapKeys<'_, K> {
MapKeys::new(&self.inner) MapKeys::new(&self.inner)
} }
} }
@ -103,7 +103,7 @@ impl<T: Deref<Target = Map>, K: Pod, V: Pod> IterableMap<K, V> for HashMap<T, K,
&self.inner &self.inner
} }
unsafe fn get(&self, key: &K) -> Result<V, MapError> { fn get(&self, key: &K) -> Result<V, MapError> {
HashMap::get(self, key, 0) HashMap::get(self, key, 0)
} }
} }
@ -373,7 +373,7 @@ mod tests {
let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); let hm = HashMap::<_, u32, u32>::new(&map).unwrap();
assert!(matches!( assert!(matches!(
unsafe { hm.get(&1, 0) }, hm.get(&1, 0),
Err(MapError::SyscallError { call, code: -1, io_error }) if call == "bpf_map_lookup_elem" && io_error.raw_os_error() == Some(EFAULT) Err(MapError::SyscallError { call, code: -1, io_error }) if call == "bpf_map_lookup_elem" && io_error.raw_os_error() == Some(EFAULT)
)); ));
} }
@ -394,10 +394,7 @@ mod tests {
}; };
let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); let hm = HashMap::<_, u32, u32>::new(&map).unwrap();
assert!(matches!( assert!(matches!(hm.get(&1, 0), Err(MapError::KeyNotFound)));
unsafe { hm.get(&1, 0) },
Err(MapError::KeyNotFound)
));
} }
fn bpf_key<T: Copy>(attr: &bpf_attr) -> Option<T> { fn bpf_key<T: Copy>(attr: &bpf_attr) -> Option<T> {
@ -432,7 +429,7 @@ mod tests {
pinned: false, pinned: false,
}; };
let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); let hm = HashMap::<_, u32, u32>::new(&map).unwrap();
let keys = unsafe { hm.keys() }.collect::<Result<Vec<_>, _>>(); let keys = hm.keys().collect::<Result<Vec<_>, _>>();
assert!(matches!(keys, Ok(ks) if ks.is_empty())) assert!(matches!(keys, Ok(ks) if ks.is_empty()))
} }
@ -477,7 +474,7 @@ mod tests {
}; };
let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); let hm = HashMap::<_, u32, u32>::new(&map).unwrap();
let keys = unsafe { hm.keys() }.collect::<Result<Vec<_>, _>>().unwrap(); let keys = hm.keys().collect::<Result<Vec<_>, _>>().unwrap();
assert_eq!(&keys, &[10, 20, 30]) assert_eq!(&keys, &[10, 20, 30])
} }
@ -505,7 +502,7 @@ mod tests {
}; };
let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); let hm = HashMap::<_, u32, u32>::new(&map).unwrap();
let mut keys = unsafe { hm.keys() }; let mut keys = hm.keys();
assert!(matches!(keys.next(), Some(Ok(10)))); assert!(matches!(keys.next(), Some(Ok(10))));
assert!(matches!(keys.next(), Some(Ok(20)))); assert!(matches!(keys.next(), Some(Ok(20))));
assert!(matches!( assert!(matches!(
@ -534,7 +531,7 @@ mod tests {
pinned: false, pinned: false,
}; };
let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); let hm = HashMap::<_, u32, u32>::new(&map).unwrap();
let items = unsafe { hm.iter() }.collect::<Result<Vec<_>, _>>().unwrap(); let items = hm.iter().collect::<Result<Vec<_>, _>>().unwrap();
assert_eq!(&items, &[(10, 100), (20, 200), (30, 300)]) assert_eq!(&items, &[(10, 100), (20, 200), (30, 300)])
} }
@ -568,7 +565,7 @@ mod tests {
}; };
let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); let hm = HashMap::<_, u32, u32>::new(&map).unwrap();
let items = unsafe { hm.iter() }.collect::<Result<Vec<_>, _>>().unwrap(); let items = hm.iter().collect::<Result<Vec<_>, _>>().unwrap();
assert_eq!(&items, &[(10, 100), (30, 300)]) assert_eq!(&items, &[(10, 100), (30, 300)])
} }
@ -602,7 +599,7 @@ mod tests {
}; };
let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); let hm = HashMap::<_, u32, u32>::new(&map).unwrap();
let mut iter = unsafe { hm.iter() }; let mut iter = hm.iter();
assert!(matches!(iter.next(), Some(Ok((10, 100))))); assert!(matches!(iter.next(), Some(Ok((10, 100)))));
assert!(matches!(iter.next(), Some(Ok((20, 200))))); assert!(matches!(iter.next(), Some(Ok((20, 200)))));
assert!(matches!( assert!(matches!(
@ -642,7 +639,7 @@ mod tests {
}; };
let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); let hm = HashMap::<_, u32, u32>::new(&map).unwrap();
let mut iter = unsafe { hm.iter() }; let mut iter = hm.iter();
assert!(matches!(iter.next(), Some(Ok((10, 100))))); assert!(matches!(iter.next(), Some(Ok((10, 100)))));
assert!(matches!( assert!(matches!(
iter.next(), iter.next(),

@ -73,7 +73,7 @@ impl<T: Deref<Target = Map>, K: Pod, V: Pod> PerCpuHashMap<T, K, V> {
} }
/// Returns a slice of values - one for each CPU - associated with the key. /// Returns a slice of values - one for each CPU - associated with the key.
pub unsafe fn get(&self, key: &K, flags: u64) -> Result<PerCpuValues<V>, MapError> { pub fn get(&self, key: &K, flags: u64) -> Result<PerCpuValues<V>, MapError> {
let fd = self.inner.deref().fd_or_err()?; let fd = self.inner.deref().fd_or_err()?;
let values = bpf_map_lookup_elem_per_cpu(fd, key, flags).map_err(|(code, io_error)| { let values = bpf_map_lookup_elem_per_cpu(fd, key, flags).map_err(|(code, io_error)| {
MapError::SyscallError { MapError::SyscallError {
@ -87,13 +87,13 @@ impl<T: Deref<Target = Map>, K: Pod, V: Pod> PerCpuHashMap<T, K, V> {
/// An iterator visiting all key-value pairs in arbitrary order. The /// An iterator visiting all key-value pairs in arbitrary order. The
/// iterator item type is `Result<(K, PerCpuValues<V>), MapError>`. /// iterator item type is `Result<(K, PerCpuValues<V>), MapError>`.
pub unsafe fn iter(&self) -> MapIter<'_, K, PerCpuValues<V>, Self> { pub fn iter(&self) -> MapIter<'_, K, PerCpuValues<V>, Self> {
MapIter::new(self) MapIter::new(self)
} }
/// An iterator visiting all keys in arbitrary order. The iterator element /// An iterator visiting all keys in arbitrary order. The iterator element
/// type is `Result<K, MapError>`. /// type is `Result<K, MapError>`.
pub unsafe fn keys(&self) -> MapKeys<'_, K> { pub fn keys(&self) -> MapKeys<'_, K> {
MapKeys::new(&self.inner) MapKeys::new(&self.inner)
} }
} }
@ -154,7 +154,7 @@ impl<T: Deref<Target = Map>, K: Pod, V: Pod> IterableMap<K, PerCpuValues<V>>
&self.inner &self.inner
} }
unsafe fn get(&self, key: &K) -> Result<PerCpuValues<V>, MapError> { fn get(&self, key: &K) -> Result<PerCpuValues<V>, MapError> {
PerCpuHashMap::get(self, key, 0) PerCpuHashMap::get(self, key, 0)
} }
} }

@ -237,7 +237,7 @@ impl Drop for Map {
pub trait IterableMap<K: Pod, V> { pub trait IterableMap<K: Pod, V> {
fn map(&self) -> &Map; fn map(&self) -> &Map;
unsafe fn get(&self, key: &K) -> Result<V, MapError>; fn get(&self, key: &K) -> Result<V, MapError>;
} }
/// Iterator returned by `map.keys()`. /// Iterator returned by `map.keys()`.
@ -317,14 +317,11 @@ impl<K: Pod, V, I: IterableMap<K, V>> Iterator for MapIter<'_, K, V, I> {
fn next(&mut self) -> Option<Self::Item> { fn next(&mut self) -> Option<Self::Item> {
loop { loop {
match self.keys.next() { match self.keys.next() {
Some(Ok(key)) => { Some(Ok(key)) => match self.map.get(&key) {
let value = unsafe { self.map.get(&key) }; Ok(value) => return Some(Ok((key, value))),
match value { Err(MapError::KeyNotFound) => continue,
Ok(value) => return Some(Ok((key, value))), Err(e) => return Some(Err(e)),
Err(MapError::KeyNotFound) => continue, },
Err(e) => return Some(Err(e)),
}
}
Some(Err(e)) => return Some(Err(e)), Some(Err(e)) => return Some(Err(e)),
None => return None, None => return None,
} }

@ -87,7 +87,7 @@ impl<T: Deref<Target = Map>, K: Pod> SockHash<T, K> {
} }
/// Returns the fd of the socket stored at the given key. /// Returns the fd of the socket stored at the given key.
pub unsafe fn get(&self, key: &K, flags: u64) -> Result<RawFd, MapError> { pub fn get(&self, key: &K, flags: u64) -> Result<RawFd, MapError> {
let fd = self.inner.deref().fd_or_err()?; let fd = self.inner.deref().fd_or_err()?;
let value = bpf_map_lookup_elem(fd, key, flags).map_err(|(code, io_error)| { let value = bpf_map_lookup_elem(fd, key, flags).map_err(|(code, io_error)| {
MapError::SyscallError { MapError::SyscallError {
@ -101,13 +101,13 @@ impl<T: Deref<Target = Map>, K: Pod> SockHash<T, K> {
/// An iterator visiting all key-value pairs in arbitrary order. The /// An iterator visiting all key-value pairs in arbitrary order. The
/// iterator item type is `Result<(K, V), MapError>`. /// iterator item type is `Result<(K, V), MapError>`.
pub unsafe fn iter(&self) -> MapIter<'_, K, RawFd, Self> { pub fn iter(&self) -> MapIter<'_, K, RawFd, Self> {
MapIter::new(self) MapIter::new(self)
} }
/// An iterator visiting all keys in arbitrary order. The iterator element /// An iterator visiting all keys in arbitrary order. The iterator element
/// type is `Result<K, MapError>`. /// type is `Result<K, MapError>`.
pub unsafe fn keys(&self) -> MapKeys<'_, K> { pub fn keys(&self) -> MapKeys<'_, K> {
MapKeys::new(&self.inner) MapKeys::new(&self.inner)
} }
} }
@ -129,7 +129,7 @@ impl<T: Deref<Target = Map>, K: Pod> IterableMap<K, RawFd> for SockHash<T, K> {
&self.inner &self.inner
} }
unsafe fn get(&self, key: &K) -> Result<RawFd, MapError> { fn get(&self, key: &K) -> Result<RawFd, MapError> {
SockHash::get(self, key, 0) SockHash::get(self, key, 0)
} }
} }

@ -71,7 +71,7 @@ impl<T: Deref<Target = Map>> SockMap<T> {
/// An iterator over the indices of the array that point to a program. The iterator item type /// An iterator over the indices of the array that point to a program. The iterator item type
/// is `Result<u32, MapError>`. /// is `Result<u32, MapError>`.
pub unsafe fn indices(&self) -> MapKeys<'_, u32> { pub fn indices(&self) -> MapKeys<'_, u32> {
MapKeys::new(&self.inner) MapKeys::new(&self.inner)
} }

@ -156,7 +156,7 @@ impl<T: Deref<Target = Map>> IterableMap<u32, StackTrace> for StackTraceMap<T> {
&self.inner &self.inner
} }
unsafe fn get(&self, index: &u32) -> Result<StackTrace, MapError> { fn get(&self, index: &u32) -> Result<StackTrace, MapError> {
self.get(index, 0) self.get(index, 0)
} }
} }

Loading…
Cancel
Save