fix(aya): Fix PerfEventArray resize logic

There was a logic bug in the previously merged patch where we
set the correctly calculated max_entries size with the original.

To fix this and prevent regressions a unit test was added.
This highlighted that the original map definition needs to be
mutated in order for the max_entries change to be properly applied.

As such, this resize logic moved out of aya::sys into aya::maps

Signed-off-by: Dave Tucker <[email protected]>

aya/src/maps/bloom_filter.rs

@@ -120,8 +120,7 @@ mod tests {
 
     #[test]
     fn test_try_from_wrong_map() {
-        // This is necessary to stop miri tripping over the PERF_EVENT_ARRAY
-        // logic and attempting to open a file to read number of online CPUs.
+        // Use any map type here other than BPF_MAP_TYPE_PERF_EVENT_ARRAY as it will trip miri
         let map = new_map(test_utils::new_obj_map::<u32>(BPF_MAP_TYPE_ARRAY));
         let map = Map::Array(map);
 

aya/src/maps/lpm_trie.rs

@@ -249,8 +249,7 @@ mod tests {
 
     #[test]
     fn test_try_from_wrong_map() {
-        // This is necessary to stop miri tripping over the PERF_EVENT_ARRAY
-        // logic and attempting to open a file to read number of online CPUs.
+        // Use any map type here other than BPF_MAP_TYPE_PERF_EVENT_ARRAY as it will trip miri
         let map = new_map(test_utils::new_obj_map::<u32>(BPF_MAP_TYPE_ARRAY));
         let map = Map::Array(map);
 

aya/src/maps/mod.rs

@@ -59,6 +59,7 @@ use std::{
     ptr,
 };
 
+use aya_obj::generated::bpf_map_type;
 use libc::{getrlimit, rlim_t, rlimit, RLIMIT_MEMLOCK, RLIM_INFINITY};
 use log::warn;
 use obj::maps::InvalidMapTypeError;
@@ -172,6 +173,10 @@ pub enum MapError {
     #[error("the program is not loaded")]
     ProgramNotLoaded,
 
+    /// An IO error occurred
+    #[error(transparent)]
+    IoError(#[from] io::Error),
+
     /// Syscall failed
     #[error(transparent)]
     SyscallError(#[from] SyscallError),
@@ -550,12 +555,30 @@ pub struct MapData {
 impl MapData {
     /// Creates a new map with the provided `name`
     pub fn create(
-        obj: obj::Map,
+        mut obj: obj::Map,
         name: &str,
         btf_fd: Option<BorrowedFd<'_>>,
     ) -> Result<Self, MapError> {
         let c_name = CString::new(name).map_err(|_| MapError::InvalidName { name: name.into() })?;
 
+        // BPF_MAP_TYPE_PERF_EVENT_ARRAY's max_entries should not exceed the number of
+        // CPUs.
+        //
+        // By default, the newest versions of Aya, libbpf and cilium/ebpf define `max_entries` of
+        // `PerfEventArray` as `0`, with an intention to get it replaced with a correct value
+        // by the loader.
+        //
+        // We allow custom values (potentially coming either from older versions of aya-ebpf or
+        // programs written in C) as long as they don't exceed the number of CPUs.
+        //
+        // Otherwise, when the value is `0` or too large, we set it to the number of CPUs.
+        if obj.map_type() == bpf_map_type::BPF_MAP_TYPE_PERF_EVENT_ARRAY as u32 {
+            let ncpus = nr_cpus().map_err(MapError::IoError)? as u32;
+            if obj.max_entries() == 0 || obj.max_entries() > ncpus {
+                obj.set_max_entries(ncpus);
+            }
+        };
+
         #[cfg(not(test))]
         let kernel_version = KernelVersion::current().unwrap();
         #[cfg(test)]
@@ -1077,6 +1100,25 @@ mod test_utils {
             symbol_index: None,
         })
     }
+
+    pub(super) fn new_obj_map_with_max_entries<K>(
+        map_type: bpf_map_type,
+        max_entries: u32,
+    ) -> obj::Map {
+        obj::Map::Legacy(LegacyMap {
+            def: bpf_map_def {
+                map_type: map_type as u32,
+                key_size: std::mem::size_of::<K>() as u32,
+                value_size: 4,
+                max_entries,
+                ..Default::default()
+            },
+            section_index: 0,
+            section_kind: EbpfSectionKind::Maps,
+            data: Vec::new(),
+            symbol_index: None,
+        })
+    }
 }
 
 #[cfg(test)]
@@ -1150,6 +1192,65 @@ mod tests {
         );
     }
 
+    #[test]
+    #[cfg_attr(miri, ignore = "nr_cpus() opens a file on procfs that upsets miri")]
+    fn test_create_perf_event_array() {
+        override_syscall(|call| match call {
+            Syscall::Ebpf {
+                cmd: bpf_cmd::BPF_MAP_CREATE,
+                ..
+            } => Ok(crate::MockableFd::mock_signed_fd().into()),
+            _ => Err((-1, io::Error::from_raw_os_error(EFAULT))),
+        });
+
+        let ncpus = nr_cpus().unwrap();
+
+        // Create with max_entries > ncpus is clamped to ncpus
+        assert_matches!(
+            MapData::create(test_utils::new_obj_map_with_max_entries::<u32>(
+                crate::generated::bpf_map_type::BPF_MAP_TYPE_PERF_EVENT_ARRAY,
+                65535,
+            ), "foo", None),
+            Ok(MapData {
+                obj,
+                fd,
+            }) => {
+                assert_eq!(fd.as_fd().as_raw_fd(), crate::MockableFd::mock_signed_fd());
+                assert_eq!(obj.max_entries(), ncpus as u32)
+            }
+        );
+
+        // Create with max_entries = 0 is set to ncpus
+        assert_matches!(
+            MapData::create(test_utils::new_obj_map_with_max_entries::<u32>(
+                crate::generated::bpf_map_type::BPF_MAP_TYPE_PERF_EVENT_ARRAY,
+                0,
+            ), "foo", None),
+            Ok(MapData {
+                obj,
+                fd,
+            }) => {
+                assert_eq!(fd.as_fd().as_raw_fd(), crate::MockableFd::mock_signed_fd());
+                assert_eq!(obj.max_entries(), ncpus as u32)
+            }
+        );
+
+        // Create with max_entries < ncpus is unchanged
+        assert_matches!(
+            MapData::create(test_utils::new_obj_map_with_max_entries::<u32>(
+                crate::generated::bpf_map_type::BPF_MAP_TYPE_PERF_EVENT_ARRAY,
+                1,
+            ), "foo", None),
+            Ok(MapData {
+                obj,
+                fd,
+            }) => {
+                assert_eq!(fd.as_fd().as_raw_fd(), crate::MockableFd::mock_signed_fd());
+                assert_eq!(obj.max_entries(), 1)
+            }
+        );
+    }
+
     #[test]
     #[cfg_attr(
         miri,

aya/src/sys/bpf.rs

@@ -30,7 +30,7 @@ use crate::{
         copy_instructions,
     },
     sys::{syscall, SysResult, Syscall, SyscallError},
-    util::{nr_cpus, KernelVersion},
+    util::KernelVersion,
     Btf, Pod, VerifierLogLevel, BPF_OBJ_NAME_LEN,
 };
 
@@ -46,26 +46,6 @@ pub(crate) fn bpf_create_map(
     u.map_type = def.map_type();
     u.key_size = def.key_size();
     u.value_size = def.value_size();
-    u.max_entries = def.max_entries();
-
-    // BPF_MAP_TYPE_PERF_EVENT_ARRAY's max_entries should not exceed the number of
-    // CPUs.
-    //
-    // By default, the newest versions of Aya, libbpf and cilium/ebpf define `max_entries` of
-    // `PerfEventArray` as `0`, with an intention to get it replaced with a correct value
-    // by the loader.
-    //
-    // We allow custom values (potentially coming either from older versions of aya-ebpf or
-    // programs written in C) as long as they don't exceed the number of CPUs.
-    //
-    // Otherwise, when the value is `0` or too large, we set it to the number of CPUs.
-    if def.map_type() == bpf_map_type::BPF_MAP_TYPE_PERF_EVENT_ARRAY as u32 {
-        let ncpus = nr_cpus().map_err(|e| (-1i64, e))? as u32;
-        if u.max_entries == 0 || u.max_entries > ncpus {
-            u.max_entries = ncpus;
-        }
-    };
-
     u.max_entries = def.max_entries();
     u.map_flags = def.map_flags();
 

xtask/public-api/aya.txt

@@ -1352,6 +1352,7 @@ pub aya::maps::MapError::InvalidName::name: alloc::string::String
 pub aya::maps::MapError::InvalidValueSize
 pub aya::maps::MapError::InvalidValueSize::expected: usize
 pub aya::maps::MapError::InvalidValueSize::size: usize
+pub aya::maps::MapError::IoError(std::io::error::Error)
 pub aya::maps::MapError::KeyNotFound
 pub aya::maps::MapError::OutOfBounds
 pub aya::maps::MapError::OutOfBounds::index: u32
@@ -1372,6 +1373,8 @@ impl core::convert::From<aya::maps::MapError> for aya::programs::ProgramError
 pub fn aya::programs::ProgramError::from(source: aya::maps::MapError) -> Self
 impl core::convert::From<aya_obj::maps::InvalidMapTypeError> for aya::maps::MapError
 pub fn aya::maps::MapError::from(e: aya_obj::maps::InvalidMapTypeError) -> Self
+impl core::convert::From<std::io::error::Error> for aya::maps::MapError
+pub fn aya::maps::MapError::from(source: std::io::error::Error) -> Self
 impl core::error::Error for aya::maps::MapError
 pub fn aya::maps::MapError::source(&self) -> core::option::Option<&(dyn core::error::Error + 'static)>
 impl core::fmt::Debug for aya::maps::MapError